From owner-freebsd-current@freebsd.org Tue Nov 28 13:26:21 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AFC71E56953; Tue, 28 Nov 2017 13:26:21 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.blih.net", Issuer "mail.blih.net" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id EC8A764AA8; Tue, 28 Nov 2017 13:26:20 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) by mail.blih.net (OpenSMTPD) with ESMTP id 3b2b6b5e; Tue, 28 Nov 2017 14:26:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; s=mail; bh=uQh2t/8+hXQh6T6SHB5qna9PjpM=; b=DqQFFgEOH8S3DsSWOhMvq5PZVa/E 6vOnCovRWEypmRoczT94wmLuYPVJMhoeqqXEBzaDEKCaQysU/Kx/naJZahEN6w6h LSmLH60J6G1qDydc8/oX8rpdWe9qwZYqmDGF+qTFOruP4/AZSOSjnrKVq0YiTiMG i9kNoIq3Z/X/kIk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; q=dns; s= mail; b=SYdzurtAJVfP4R0oSW52QDpyUzqFkn6FEcmK4xFCY1+YGz2CBaTOpjIb zoRVrhXiSEw1spgxdMdfIKEu4oKvLRIH5sIUyBQMdRjHpaT87Ys6CDM9XQ8hnn3r AG3hyZCF/kQZ804miXHWlB36mw5Z03eEboqagk5Tt4Ry8Jgjyks= Received: from arcadia (evadot.gandi.net [217.70.181.36]) by mail.blih.net (OpenSMTPD) with ESMTPSA id 1130363c TLS version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO; Tue, 28 Nov 2017 14:26:11 +0100 (CET) Date: Tue, 28 Nov 2017 14:26:10 +0100 From: Emmanuel Vadot To: Konstantin Belousov Cc: FreeBSD Current , freebsd-fs@freebsd.org, Rick Macklem Subject: Re: Switch vfs.nfsd.issue_delegations to TUNABLE ? Message-Id: <20171128142610.6ab535b0b8c6b5d372cd3f27@bidouilliste.com> In-Reply-To: <20171128110428.GN2272@kib.kiev.ua> References: <20171128114136.10e44d5bcfd563e9b4ab5453@bidouilliste.com> <20171128110428.GN2272@kib.kiev.ua> X-Mailer: Sylpheed 3.6.0 (GTK+ 2.24.31; amd64-portbld-freebsd12.0) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Nov 2017 13:26:21 -0000 On Tue, 28 Nov 2017 13:04:28 +0200 Konstantin Belousov 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. 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. > > > > 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 > > 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 > > --- > > 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 > > _______________________________________________ > > freebsd-fs@freebsd.org mailing list > > https://lists.freebsd.org/mailman/listinfo/freebsd-fs > > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org" > _______________________________________________ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" -- Emmanuel Vadot