Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\))
Subject: Re: [PATCH RFC] NFSD: synchronously unhash a file on NFSv4 CLOSE
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <18B8C7A0-CDF9-4B5F-AABF-43189CE84CEC@oracle.com>
Date:   Tue, 15 Sep 2020 09:18:31 -0400
Cc:     Trond Myklebust <trond.myklebust@primarydata.com>,
        Jeff Layton <jlayton@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <25B85F11-E8AA-444B-85F8-039135A43D39@oracle.com>
References: <159976711218.1334.14422329830210182280.stgit@klimt.1015granger.net>
 <20200914205127.GD30007@fieldses.org>
 <18B8C7A0-CDF9-4B5F-AABF-43189CE84CEC@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Sep 14, 2020, at 4:53 PM, Chuck Lever <chuck.lever@oracle.com> =
wrote:
>=20
>=20
>=20
>> On Sep 14, 2020, at 4:51 PM, J. Bruce Fields <bfields@fieldses.org> =
wrote:
>>=20
>> On Thu, Sep 10, 2020 at 03:45:12PM -0400, Chuck Lever wrote:
>>> I recently observed a significant slowdown in a long-running
>>> test on NFSv4.0 mounts.
>>>=20
>>> An OPEN for write seems to block the NFS server from offering
>>> delegations on that file for a few seconds. The problem is that
>>> when that file is closed, the filecache retains an open-for-write
>>> file descriptor until the laundrette runs. That keeps the inode's
>>> i_writecount positive until the cached file is finally unhashed
>>> and closed.
>>>=20
>>> Force the NFSv4 CLOSE logic to call filp_close() to eliminate
>>> the underlying cached open-for-write file as soon as the client
>>> closes the file.
>>>=20
>>> This minor change claws back about 80% of the performance
>>> regression.
>>=20
>> That's really useful to know.  But mainly this makes me think that
>> nfsd4_check_conflicting_opens() is wrong.
>>=20
>> I'm trying to determine whether a given file has a non-nfsd writer by
>> counting the number of write opens nfsd holds on a given file and
>> subtracting that from i_writecount.
>>=20
>> But the way I'm counting write opens probably isn't correct.
>=20
> I entertained that possibility, but I couldn't figure out
> a better way to count opens-for-write.

Also, the regression started long before nfsd4_check_conflicting_opens
was added.


>> --b.
>>=20
>>>=20
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> fs/nfsd/nfs4state.c |    8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>=20
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index 3ac40ba7efe1..0b3059b8b36c 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -613,10 +613,14 @@ static void __nfs4_file_put_access(struct =
nfs4_file *fp, int oflag)
>>> 		if (atomic_read(&fp->fi_access[1 - oflag]) =3D=3D 0)
>>> 			swap(f2, fp->fi_fds[O_RDWR]);
>>> 		spin_unlock(&fp->fi_lock);
>>> -		if (f1)
>>> +		if (f1) {
>>> +			=
nfsd_file_close_inode_sync(locks_inode(f1->nf_file));
>>> 			nfsd_file_put(f1);
>>> -		if (f2)
>>> +		}
>>> +		if (f2) {
>>> +			=
nfsd_file_close_inode_sync(locks_inode(f2->nf_file));
>>> 			nfsd_file_put(f2);
>>> +		}
>>> 	}
>>> }
>=20
> --
> Chuck Lever

--
Chuck Lever