Re: [Virtio-fs] [for-6.1 v3 3/3] virtiofsd: Add support for FUSE_SYNCFS request

[Vivek Goyal <vgoyal@redhat.com>]

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@kaod.org> 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