On Tue, May 11, 2021 at 05:08:42PM +0200, Miklos Szeredi wrote: > On Tue, May 11, 2021 at 4:49 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > 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. > > Just setting SB_BORN sounds much simpler. What's the disadvantage? I was little hesitant to set it directly because no other filesystem seems to be doing it. Hence I assumed that VFS expects filesystems to not set SB_BORN. But I do agree that setting SB_BORN in automount code is much simpler solution. Thanks Vivek
