On Tue, May 11, 2021 at 08:54:09AM -0400, Vivek Goyal wrote: > On Tue, May 11, 2021 at 02:31:14PM +0200, Miklos Szeredi wrote: > > On Mon, May 10, 2021 at 5:55 PM Greg Kurz <groug@xxxxxxxx> wrote: > > > > > > Honor the expected behavior of syncfs() to synchronously flush all data > > > and metadata on linux systems. Simply loop on all known submounts and > > > call syncfs() on them. > > > > Why not pass the submount's root to the server, so it can do just one > > targeted syncfs? > > > > E.g. somehting like this in fuse_sync_fs(): > > > > args.nodeid = get_node_id(sb->s_root->d_inode); > > Hi Miklos, > > I think current proposal was due to lack of full understanding on my part. > I was assuming we have one super block in client and that's not the case > looks like. For every submount, we will have another superblock known > to vfs, IIUC. That means when sync() happens, we will receive ->syncfs() > for each of those super blocks. And that means file server does not > have to keep track of submounts explicitly and it will either receive > a single targeted SYNCFS (for the case of syncfs(fd)) or receive > multile SYNCFS calls (one for each submount when sync() is called). Tried sync() with submounts enabled and we are seeing a SYNCFS call only for top level super block and not for submounts. Greg noticed that it probably is due to the fact that iterate_super() skips super blocks which don't have SB_BORN flag set. Only vfs_get_tree() seems to set SB_BORN and for our submounts we are not calling vfs_get_tree(), hence SB_BORN is not set. NFS seems to call vfs_get_tree() and hence SB_BORN must be set for submounts. Maybe we need to modify virtio_fs_get_tree() so that it can deal with mount as well as submounts and then fuse_dentry_automount() should probably call vfs_get_tree() and that should set SB_BORN and hopefully sync() will work with it. Greg is planning to give it a try. Does it sound reasonable. Thanks Vivek
