Re: Switch vfs.nfsd.issue_delegations to TUNABLE ?

From: Emmanuel Vadot <manu_at_bidouilliste.com>
Date: Wed, 29 Nov 2017 14:40:40 +0100
Hello Rick,

On Tue, 28 Nov 2017 23:18:57 +0000
Rick Macklem <rmacklem_at_uoguelph.ca> wrote:

> Did my usual and forgot to attach it. Here's the patch, rick
> 
> ________________________________________
> From: Rick Macklem <rmacklem_at_uoguelph.ca>
> Sent: Tuesday, November 28, 2017 6:17:13 PM
> To: Emmanuel Vadot
> Cc: Konstantin Belousov; FreeBSD Current; freebsd-fs_at_freebsd.org; Rick Macklem
> Subject: Re: Switch vfs.nfsd.issue_delegations to TUNABLE ?
> 
> I think the attached patch should fix your performance problem.
> It simply checks for nfsrv_delegatecnt != 0 before doing all the
> locking stuff in nfsrv_checkgetattr().
> 
> If this fixes your performance problem (with delegations disabled),
> I'll probably add a separate counter for write delegations and
> use atomics to increment/decrement it so that it is SMP safe without
> acquiring any lock.
> 
> If you can test this, please let me know how it goes? rick

 I haven't test by I can say that it will work, I actually wondered at
first doing that. The problem with this patch is what I tried to
describe in my first and following mails, since you can turn on and off
delegation you can still have delegation (so nfsrv_delegatecnt > 0)
even if the sysctl is == 0. That's why I went to the tunable way, seems
cleaner to me as disabling delegation doesn't really disable them for
current client.

> ________________________________________
> From: Rick Macklem <rmacklem_at_uoguelph.ca>
> Sent: Tuesday, November 28, 2017 2:09:51 PM
> To: Emmanuel Vadot
> Cc: Konstantin Belousov; FreeBSD Current; freebsd-fs_at_freebsd.org; Rick Macklem
> Subject: Re: Switch vfs.nfsd.issue_delegations to TUNABLE ?
> 
> Emmanuel Vadot wrote:
> >I wrote:
> >> Since it defaults to "disabled", I don't see why a tunable would be necessary?
> >> (Just do nothing and delegations don't happen. If you want the server
> >>  to issue delegations, then use the sysctl to turn them on. If you want to turn
> >>  them off again at the server, reboot the server without setting the sysctl.)
> >
> >If you need to reboot to make things working again without delegation
> >this shouldn't be a sysctl.
> Turning them off without rebooting doesn't break anything.
> I just wrote it that way since you seemed to want to use a tunable, which
> implied rebooting was your preference.
> > > > >  The reason behind it is recent problem at work on some on our filer
> > > > > related to NFS.
> > > > >  We use NFSv4 without delegation as we never been able to have good
> > > > > performance with FreeBSD server and Linux client (we need to do test
> > > > > again but that's for later).
> > Delegations are almost never useful, especially with Linux clients.
> Emmanuel Vadot wrote:
> >Can you elaborate ? Reading what delegation are I see that this is
> >exactly what I'm looking for to have better performance with NFS for my
> >workload (I only have one client per mount point).
> Delegations allow the client to do Opens locally without contacting the
> server. Unless, the delegation is a write delegation, this only applied
> to read-only delegations. Also, since most implementors couldn`t agree
> on how to check permissions via the delegation, most client implementations
> still do an Access check at open, which is almost as much overhead as the
> Open RPC. (For example, Solaris servers always specified an empty ACE in the
> delegation, forcing the client to do an Access. I have no idea what the
> current Linux serveréclient does. If you capture packets when a Linux
> client is mounted with delegations enabled, you could look for RPCs like Access when
> are Opened multiple times. If you see them, then delegations aren`t saving RPCs.
> Also, they are `per file`, so are only useful if the client is Opening the
> same file multiple times.
> Further, if another client Opens the same file and the first client got a Write
> delegation, then the write delegation must be recalled, which is a lot of
> overhead and one of the few cases where the FreeBSD server must exclusively
> lock the state lists, forcing all other RPCs related to Open, Close to wait.
> 
> They sounded good in theory and might have worked well if the implementors
> had agreed to do them, but that didnèt happen. (Companies pay for servers, but the
> clients donèt get funded so delegation support in the clients are lacking. I tried
> to make them useful in the FreeBSD client, but Ièm not sure I succeeded.)
> 
> > [stuff snipped]
> If I understood your original post, you have a performance problem caused
> by lock contention, where the server grabs the state lock to check for delegations
> for every Getattr from a client.
> 
> As below, I think the fix is to add code to check for no delegations issued that
> does not require acquisition of the state lock.
> 
> Btw, large numbers of Getattrs will not perform well with delegations.
> (Again, the client should be able to do Getattr operations locally in the
>  client when it has a delegation for the file, but if the client is not doing that...)
> 
> I wrote:
> >
> > Having a per-mount version of this would be overkill, I think. It would have to
> > disable callbacks on the mount point, since there is no way for a client to say
> > "don't give me delegations" except disabling callbacks, which the server
> > requires for delegation issue.
> > [stuff snipped]
> > The case where there has never been delegations issued will result in an
> > empty delegation queue. Maybe this case can be handled without acquiring
> > the "global NFSv4 state lock", which is what sounds like is causing the
> > performance issue. Maybe a global counter of how many delegations are
> > issued that is handled by atomic ops.
> > --> If it is 0, nfsrv_checkgetattr() can just return without acquiring the lock.
> >
> > I'll take a look at this, rick
> >
> rick


-- 
Emmanuel Vadot <manu_at_bidouilliste.com> <manu_at_freebsd.org>
Received on Wed Nov 29 2017 - 12:40:51 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:13 UTC