On 4/18/22 6:29 PM, Jeff Layton wrote:
On Mon, 2022-04-18 at 18:25 +0800, Xiubo Li wrote:
On 4/18/22 6:15 PM, Jeff Layton wrote:
On Mon, 2022-04-11 at 17:34 +0800, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>
From the posix and the initial statx supporting commit comments,
the AT_STATX_DONT_SYNC is a lightweight stat flag and the
AT_STATX_FORCE_SYNC is a heaverweight one. And also checked all
the other current usage about these two flags they are all doing
the same, that is only when the AT_STATX_FORCE_SYNC is not set
and the AT_STATX_DONT_SYNC is set will they skip sync retriving
the attributes from storage.
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
fs/ceph/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 6788a1f88eb6..1ee6685def83 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2887,7 +2887,7 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
return -ESTALE;
/* Skip the getattr altogether if we're asked not to sync */
- if (!(flags & AT_STATX_DONT_SYNC)) {
+ if ((flags & AT_STATX_SYNC_TYPE) != AT_STATX_DONT_SYNC) {
err = ceph_do_getattr(inode,
statx_to_caps(request_mask, inode->i_mode),
flags & AT_STATX_FORCE_SYNC);
I don't get it.
The only way I can see that this is a problem is if someone sent down a
mask with both DONT_SYNC and FORCE_SYNC set in it, and in that case I
don't see that ignoring FORCE_SYNC would be wrong...
There has 3 cases for the flags:
case1: flags & AT_STATX_SYNC_TYPE == 0
case2: flags & AT_STATX_SYNC_TYPE == AT_STATX_DONT_SYNC
case3: flags & AT_STATX_SYNC_TYPE == AT_STATX_DONT_SYNC |
AT_STATX_FORCE_SYNC
Only in case2, which is only the DONT_SYNC bit is set, will ignore
calling ceph_do_getattr() here. And for case3 it will ignore the
DONT_SYNC bit.
Sure, but the patch doesn't functionally change the behavior of the
code. It may make the condition more idiomatic to read, but I don't
think there is a bug here.
- if (!(flags & AT_STATX_DONT_SYNC)) {
For example, in both case2 and case3 the above condition is false, right
? That means for case3 it will ignore the FORCE_SYNC always.
+ if ((flags & AT_STATX_SYNC_TYPE) != AT_STATX_DONT_SYNC) {
For exmaple in case2 the above condition is false and then it will skip
calling the ceph_do_getattr(). And in case3 the above condition is true,
it will call the ceph_do_getattr(..., flags & FORCE_SYNC).
The logic changed, right ?
-- Xiubo
