fuse: Allow stale time info for permission getattr by cding-ddn · Pull Request #150 · DDNStorage/linux

cding-ddn · 2026-05-06T09:35:26Z

This allows the FUSE server to acquire a weaker lock mode and reduce
lock contention, specifically for shared directory file operations.

hbirth · 2026-05-07T07:38:16Z

LGTM
helps for not losing the lock for no reason

cding-ddn · 2026-05-07T12:28:51Z

Changed FUSE_GETATTR_PERM_CHECK to (1 << 30) to avoid fuse_lowlevel.c:1617:62: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'

bsbernd · 2026-05-07T13:35:43Z


 	forget_all_cached_acls(inode);
-	return fuse_do_getattr(inode, NULL, NULL);
+	return fuse_do_getattr(inode, NULL, NULL, true);


So here you basically want to pass in mask with STATX_MODE

And actually that entire code path here will not be used if the file system supports statx

bsbernd · 2026-05-07T13:43:59Z

 			}
 		} else {
-			err = fuse_do_getattr(inode, stat, file);
+			err = fuse_do_getattr(inode, stat, file, false);


A plain stat() shall not get time stamps?

Called in code path of
struct inode_operations::fuse_getattr

why not? ... I'm more worried if we can somehow disable this behavior

So you call 'stat file' and get random time stamp results?

I don't think that manually called stat or readdir is the issue, but more like getattr from fuse in the middle of the code when it believes it needs to update something like mode for security reasons. And that is where statx then is the way to go - it was created exactly for that purpose. https://lkml.org/lkml/2016/11/17/382 , see

(3) Lightweight stat: Ask for just those details of interest, and allow a netfs (such as NFS) to approximate anything not of interest, possibly without going to the server [Trond Myklebust, Ulrich Drepper, Andreas Dilger].

So you call 'stat file' and get random time stamp results?

If that is what the customer wants, yes

And why would the customer want to have random time stamps?

Actually I read it wrong, here it false gets all attributes. Still, do don't it, but use statx

bsbernd

@cding-ddn I has to done like this

dir.c:int fuse_update_attributes(struct inode *inode, struct file *file, u32 mask)
file.c:		err = fuse_update_attributes(inode, iocb->ki_filp, STATX_SIZE);
file.c:		err = fuse_update_attributes(mapping->host, file,
file.c:	err = fuse_update_attributes(inode, file, STATX_SIZE);
file.c:		retval = fuse_update_attributes(inode, file, STATX_SIZE);
fuse_i.h:int fuse_update_attributes(struct inode *inode, struct file *file, u32 mask);
readdir.c:		int err = fuse_update_attributes(inode, file, STATX_MTIME);

Bad hacks are not needed

bsbernd · 2026-05-07T13:55:48Z

I guess that actually the right flags are already mostly requested, some minor changes are needed. And then redfsd needs to support statx instead of a hack.

bsbernd · 2026-05-07T18:47:38Z

 			 * Refresh and recalculate.
 			 */
-			ret = fuse_do_getattr(inode, NULL, file);
+			ret = fuse_do_getattr(inode, NULL, file, false);


hbirth · 2026-05-08T05:57:52Z

I actually thought that statx is not possible for some reason and that was the reason to do it like this. Implicit communication doesn't really work, I know. But if the whole reason is, that we don't want to support statx, then maybe we should and I agree with @bsbernd
What was the reason that redfsd doesn't support statx?
IIRC there were some changes in libfuse regarding statx that were made after redfsd was already done.

hbirth · 2026-05-08T12:24:57Z

@cding-ddn we can probably close this

This allows the FUSE server to acquire a weaker lock mode and reduce lock contention, specifically for shared directory file operations.

cding-ddn · 2026-05-09T10:16:29Z

@bsbernd @hbirth
Changed to statx per your suggestion. Could you review again?

bsbernd · 2026-05-09T22:26:19Z


 	forget_all_cached_acls(inode);
-	return fuse_do_getattr(inode, NULL, NULL);
+	return fuse_perm_do_statx(inode, NULL);


I think that can be much simpler, look at the caller of fuse_perm_getattr, i.e. fuse_permission

u32 perm_mask = STATX_MODE | STATX_UID | STATX_GID;

How about this

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 67581ce277b5..2ae36475e722 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1184,7 +1184,7 @@ static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr) } static int fuse_do_statx(struct inode *inode, struct file *file, - struct kstat *stat) + struct kstat *stat, int sx_mask) { int err; struct fuse_attr attr; @@ -1206,7 +1206,7 @@ static int fuse_do_statx(struct inode *inode, struct file *file, } /* For now leave sync hints as the default, request all stats. */ inarg.sx_flags = 0; - inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME; + inarg.sx_mask = sx_mask; args.opcode = FUSE_STATX; args.nodeid = get_node_id(inode); args.in_numargs = 1; @@ -1314,9 +1314,8 @@ static int fuse_update_get_attr(struct inode *inode, struct file *file, if (sync) { forget_all_cached_acls(inode); - /* Try statx if BTIME is requested */ - if (!fc->no_statx && (request_mask & ~STATX_BASIC_STATS)) { - err = fuse_do_statx(inode, file, stat); + if (!fc->no_statx) { + err = fuse_do_statx(inode, file, stat, request_mask); if (err == -ENOSYS) { fc->no_statx = 1; err = 0; @@ -1478,13 +1477,13 @@ static int fuse_access(struct inode *inode, int mask) return err; } -static int fuse_perm_getattr(struct inode *inode, int mask) +static int fuse_perm_getattr(struct inode *inode, int mask, int perm_mask) { if (mask & MAY_NOT_BLOCK) return -ECHILD; forget_all_cached_acls(inode); - return fuse_do_getattr(inode, NULL, NULL); + return fuse_update_get_attr(inode, NULL, NULL, perm_mask, AT_STATX_FORCE_SYNC); } /* @@ -1506,6 +1505,7 @@ static int fuse_permission(struct mnt_idmap *idmap, struct fuse_conn *fc = get_fuse_conn(inode); bool refreshed = false; int err = 0; + int perm_mask = STATX_MODE | STATX_UID | STATX_GID; if (fuse_is_bad(inode)) return -EIO; @@ -1519,13 +1519,12 @@ static int fuse_permission(struct mnt_idmap *idmap, if (fc->default_permissions || ((mask & MAY_EXEC) && S_ISREG(inode->i_mode))) { struct fuse_inode *fi = get_fuse_inode(inode); - u32 perm_mask = STATX_MODE | STATX_UID | STATX_GID; if (perm_mask & READ_ONCE(fi->inval_mask) || time_before64(fi->i_time, get_jiffies_64())) { refreshed = true; - err = fuse_perm_getattr(inode, mask); + err = fuse_perm_getattr(inode, mask, perm_mask); if (err) return err; } @@ -1538,7 +1537,7 @@ static int fuse_permission(struct mnt_idmap *idmap, attributes. This is also needed, because the root node will at first have no permissions */ if (err == -EACCES && !refreshed) { - err = fuse_perm_getattr(inode, mask); + err = fuse_perm_getattr(inode, mask, perm_mask); if (!err) err = generic_permission(&nop_mnt_idmap, inode, mask); @@ -1555,7 +1554,7 @@ static int fuse_permission(struct mnt_idmap *idmap, if (refreshed) return -EACCES; - err = fuse_perm_getattr(inode, mask); + err = fuse_perm_getattr(inode, mask, perm_mask); if (!err && !(inode->i_mode & S_IXUGO)) return -EACCES; }

Sorry, only noticed past midnight when I was just on my way to bed - the above is not even compilation tested.

Basically this passes in the request mask to fuse_statx_to_attr() and uses that function in fuse_perm_getattr() via fuse_update_get_attr().
I don't know if Miklos will accept it like this, but I don't think he can reject it entirely either, because as I said before, one of the reasons to create statx was the network file system case with a request mask - the file system is obliged to fill in the requested arguments, but is free to fill in more.

With "much simpler" I actually mean "without code dup".

Yes, but that may cause problem when we cherry-pick upstream kernel changes later.

I mean my change intentionally does not modify the existing kernel routine signature.

Obviously we need to send my suggestion upstream.

@bsbernd The suggested code fails to improve performance; fuse_do_statx() does not invoke redfs_change_attributes() for the requested mask STATX_MODE | STATX_UID | STATX_GID.

cding-ddn · 2026-05-18T15:36:56Z

We are going to use #151
close this one.

cding-ddn requested review from bsbernd, hbirth and yongzech May 6, 2026 09:35

hbirth previously approved these changes May 7, 2026

View reviewed changes

yongzech previously approved these changes May 7, 2026

View reviewed changes

cding-ddn dismissed stale reviews from yongzech and hbirth via 96a9e59 May 7, 2026 12:24

cding-ddn force-pushed the getattr-noble-6.8.0-58.60 branch from 437f393 to 96a9e59 Compare May 7, 2026 12:24

bsbernd reviewed May 7, 2026

View reviewed changes

Comment thread fs/fuse/dir.c

bsbernd reviewed May 7, 2026

View reviewed changes

bsbernd requested changes May 7, 2026

View reviewed changes

bsbernd reviewed May 7, 2026

View reviewed changes

fuse: Allow stale time info for permission getattr

4d8c563

This allows the FUSE server to acquire a weaker lock mode and reduce lock contention, specifically for shared directory file operations.

cding-ddn force-pushed the getattr-noble-6.8.0-58.60 branch from 96a9e59 to 4d8c563 Compare May 9, 2026 10:13

cding-ddn changed the title ~~fuse: add FUSE_GETATTR_PERM_CHECK flag~~ fuse: Allow stale time info for permission getattr May 9, 2026

hbirth requested review from bsbernd and hbirth May 9, 2026 16:15

hbirth approved these changes May 9, 2026

View reviewed changes

bsbernd reviewed May 9, 2026

View reviewed changes

cding-ddn closed this May 18, 2026

Conversation

cding-ddn commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hbirth commented May 7, 2026

Uh oh!

cding-ddn commented May 7, 2026

Uh oh!

Uh oh!

bsbernd May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsbernd May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsbernd left a comment

Choose a reason for hiding this comment

Uh oh!

bsbernd commented May 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbirth commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hbirth commented May 8, 2026

Uh oh!

cding-ddn commented May 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cding-ddn commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cding-ddn commented May 6, 2026 •

edited

Loading

bsbernd May 7, 2026 •

edited

Loading

bsbernd May 7, 2026 •

edited

Loading

hbirth commented May 8, 2026 •

edited

Loading