On 4/29/2025 10:23 PM, Bernd Schubert wrote:
I'm kind of ok with adding my Reviewed-by line, but better not in advance.
Miklos also didn't add his Reviewed-by.
Thanks the reminder, and I will remove these lines from new patch.
Issue with the patch is that it does not apply
# b4 am 62a4a1dd-124a-4576-9138-cdf1d4cbb638@xxxxxxx
# bschubert2@imesrv6 linux.git>git am --3way ./v2_20250421_gwu_fs_fuse_fix_race_between_concurrent_setattr_from_multiple_nodes.mbx
Applying: fs/fuse: fix race between concurrent setattr from multiple nodes
error: corrupt patch at line 11
error: could not build fake ancestor
And then looking at the patch, I think all tabs are converted to
spaces - doesn't work.
Thanks, and format will be updated from new patch.
Also, I will invoke checkpatch.pl to verify the format.
---
fs/fuse/dir.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 83ac192e7fdd..0cc5a07a42e6 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1946,6 +1946,8 @@ int fuse_do_setattr(struct mnt_idmap *idmap,
struct dentry *dentry,
int err;
bool trust_local_cmtime = is_wb;
bool fault_blocked = false;
+ bool invalidate_attr = false;
+ u64 attr_version;
if (!fc->default_permissions)
attr->ia_valid |= ATTR_FORCE;
@@ -2030,6 +2032,8 @@ int fuse_do_setattr(struct mnt_idmap *idmap,
struct dentry *dentry,
if (fc->handle_killpriv_v2 && !capable(CAP_FSETID))
inarg.valid |= FATTR_KILL_SUIDGID;
}
+
+ attr_version = fuse_get_attr_version(fm->fc);
fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
err = fuse_simple_request(fm, &args);
if (err) {
@@ -2055,9 +2059,17 @@ int fuse_do_setattr(struct mnt_idmap *idmap,
struct dentry *dentry,
/* FIXME: clear I_DIRTY_SYNC? */
}
+ if (attr_version != 0 && fi->attr_version > attr_version)
+ /* Applying attributes, for example for
fsnotify_change() */
+ invalidate_attr = true;
+
fuse_change_attributes_common(inode, &outarg.attr, NULL,
ATTR_TIMEOUT(&outarg),
fuse_get_cache_mask(inode), 0);
We could also specific 0 as timeout.
fuse_change_attributes_common(inode, &outarg.attr, NULL,
invalidate_attr ? 0 : ATTR_TIMEOUT(&outarg),
fuse_get_cache_mask(inode), 0);
Yes, we can set i_time to 0 here, and next getattr call will set "sync"
to true from line 1327 instead of line 1325 in patch.
1300 static int fuse_update_get_attr(struct mnt_idmap *idmap, struct
inode *inode,
...
1324 else if (request_mask & inval_mask & ~cache_mask)
1325 sync = true;
1326 else
1327 sync = time_before64(fi->i_time, get_jiffies_64());
1328
1329 if (sync) {
...
+
+ if (invalidate_attr)
+ fuse_invalidate_attr(inode);
And then these lines wouldn't be needed.
Ack.
+
oldsize = inode->i_size;
/* see the comment in fuse_change_attributes() */
if (!is_wb || is_truncate)
(END)
Regards
Guang Yuan Wu
Thanks,
Bernd
RegardsGuang Yuan
