Switch vfs.nfsd.issue_delegations to TUNABLE ?
Konstantin Belousov
kostikbel at gmail.com
Tue Nov 28 13:40:20 UTC 2017
On Tue, Nov 28, 2017 at 02:26:10PM +0100, Emmanuel Vadot wrote:
> On Tue, 28 Nov 2017 13:04:28 +0200
> Konstantin Belousov <kostikbel at gmail.com> wrote:
>
> > On Tue, Nov 28, 2017 at 11:41:36AM +0100, Emmanuel Vadot wrote:
> > >
> > > Hello,
> > >
> > > I would like to switch the vfs.nfsd.issue_delegations sysctl to a
> > > tunable.
> > > 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). We recently see all of our filers with NFS
> > > enabled perform pourly, resulting in really bad performance on the
> > > service.
> > > After doing some analyze with pmcstat we've seen that we spend ~50%
> > > of our time in lock delay, called by nfsrv_checkgetattr (See [1]).
> > > This function is only usefull when using delegation, as it search for
> > > some write delegations issued to client, but it's called anyway when
> > > there so no delegation and cause a lot of problem when having a lot of
> > > activities.
> > > We've patched the kernel with the included diff and now everything is
> > > fine (tm) (See [2]).
> > > The problem for upstreaming this patch is that since issue_delegations
> > > is a sysctl we cannot know if the checkgetattr called is needed, hence
> > > the question to switch it to a TUNABLE. Also maybe some other code path
> > > could benefit from it, I haven't read all the source of nfsserver yet.
> > >
> > > Note that I won't MFC the changes as it would break POLA.
> > Perhaps make nodeleg per-mount flag ?
> > The you can check it safely by dereferencing vp->v_mount->mnt_data without
> > acquiring any locks, while the vnode lock is owned and the vnode is not
> > reclaimed.
>
> That won't work with current code.
Why ?
> Currently if you have delegation enabled and connect one client to a
> mountpoint, then disable delegation, the current client will still have
> delegation while new ones will not. I have not tested restarting nfsd in
> this situation but I suspect that client will behave badly. This is a
> another +1 for making it a tunable I think.
It is up to the filesystem to handle remount, in particular, fs can disable
changing mount options on mount upgrade if the operation is not supported.
In other words, you would do
mount -o nodeleg ... /mnt
and
mount -u -o nonodeleg ... /mnt
needs to return EINVAL.
>
> > >
> > > Cheers,
> > >
> > > [1] https://people.freebsd.org/~manu/m8.svg
> > > [2] https://people.freebsd.org/~manu/m8-new.svg
> > >
> > > >From 0cba277f406d3ccf3c9e943a3d4e17b529e31c89 Mon Sep 17 00:00:00 2001
> > > From: Emmanuel Vadot <manu at freebsd.org>
> > > Date: Fri, 24 Nov 2017 11:17:18 +0100
> > > Subject: [PATCH 2/4] Do not call nfsrv_checkgetttr if delegation isn't
> > > enable.
> > >
> > > Signed-off-by: Emmanuel Vadot <manu at freebsd.org>
> > > ---
> > > sys/fs/nfsserver/nfs_nfsdserv.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c
> > > b/sys/fs/nfsserver/nfs_nfsdserv.c index 8102c5810a9..8daf0142360 100644
> > > --- a/sys/fs/nfsserver/nfs_nfsdserv.c
> > > +++ b/sys/fs/nfsserver/nfs_nfsdserv.c
> > > @@ -54,6 +54,7 @@ extern struct timeval nfsboottime;
> > > extern int nfs_rootfhset;
> > > extern int nfsrv_enable_crossmntpt;
> > > extern int nfsrv_statehashsize;
> > > +extern int nfsrv_issuedelegs;
> > > #endif /* !APPLEKEXT */
> > >
> > > static int nfs_async = 0;
> > > @@ -240,7 +241,7 @@ nfsrvd_getattr(struct nfsrv_descript *nd, int
> > > isdgram, if (nd->nd_flag & ND_NFSV4) {
> > > if (NFSISSET_ATTRBIT(&attrbits,
> > > NFSATTRBIT_FILEHANDLE)) nd->nd_repstat = nfsvno_getfh(vp, &fh, p);
> > > - if (!nd->nd_repstat)
> > > + if (nd->nd_repstat == 0 && nfsrv_issuedelegs
> > > == 1) nd->nd_repstat = nfsrv_checkgetattr(nd, vp,
> > > &nva, &attrbits, nd->nd_cred, p);
> > > if (nd->nd_repstat == 0) {
> > > --
> > > 2.14.2
> > >
> > >
> > > --
> > > Emmanuel Vadot <manu at bidouilliste.com> <manu at freebsd.org>
> > > _______________________________________________
> > > freebsd-fs at freebsd.org mailing list
> > > https://lists.freebsd.org/mailman/listinfo/freebsd-fs
> > > To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"
> > _______________________________________________
> > freebsd-current at freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-current
> > To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>
>
> --
> Emmanuel Vadot <manu at bidouilliste.com> <manu at freebsd.org>
More information about the freebsd-fs
mailing list