Re: [PATCH 068/104] virtiofsd: passthrough_ll: control readdirplus

[Vivek Goyal]

On Fri, Jan 10, 2020 at 04:30:01PM +0100, Miklos Szeredi wrote:
> On Fri, Jan 10, 2020 at 4:18 PM Daniel P. Berrangé <address@hidden> wrote:
> >
> > On Fri, Jan 10, 2020 at 04:13:08PM +0100, Miklos Szeredi wrote:
> > > On Fri, Jan 10, 2020 at 4:04 PM Dr. David Alan Gilbert
> > > <address@hidden> wrote:
> > > >
> > > > * Daniel P. Berrangé (address@hidden) wrote:
> > > > > On Thu, Dec 12, 2019 at 04:38:28PM +0000, Dr. David Alan Gilbert 
> > > > > (git) wrote:
> > > > > > From: Miklos Szeredi <address@hidden>
> > > > > >
> > > > >
> > > > > What is readdirplus and what do we need a command line option to
> > > > > control it ? What's the user benefit of changing the setting ?
> > > >
> > > > cc'ing Miklos who understands this better than me.
> > > >
> > > > My understanding is that readdirplus is a heuristic inherited from NFS
> > > > where when you iterate over the directory you also pick up stat() data
> > > > for each entry in the directory.  You then cache that stat data
> > > > somewhere.
> > > > The Plus-ness is that a lot of directory operations involve you stating
> > > > each entry (e.g. to figure out if you can access it etc) so rolling it
> > > > into one op avoids the separate stat.  The unplus-ness is that it's an
> > > > overhead and I think changes some of the caching behaviour.
> > >
> > > Yeah, so either may give better performance and it's hard to pick a
> > > clear winner.  NFS also has an option to control this.
> >
> > IIUC from the man page, the NFS option for controlling this is a client
> > side mount option. This makes sense as only the client really has knowledge
> > of whether its workload will benefit.
> >
> > With this in mind, should the readdirplus control for virtio-fs also be a
> > guest mount option instead of a host virtiofsd CLI option ? The guest admin
> > seems best placed to know whether their workload will benefit or not.
> 
> Definitely.   In fact other options, e.g. ones that control caching,
> should probably also be client side (cache=XXX, writeback,
> timeout=XXX, etc).

I am not sure about cache options. So if we want to share a directory
between multiple guests with stronger coherency (cache=none), then admin
should decide that cache=always/auto is not supported on this export.

Also, how will one client know whether there are other clients same
directory with strong coherency requirements and it should use cache=none
instead of cache=always/auto.

Having said that, it also makes sense that client knows its workoad
and can decide if cache=auto works best for it and use that instead.

May be we need both client and server side options. Client will request
certain cache=xxx options and server can deny these if admin decides
not to enable that option for that particular mount.

For example, if admin decides that we can only support cache=none on
this particular dir due to other guest sharing it, then daemon should
be able to deny cache=auto/always requests from client.

Thanks
Vivek