> On Sep 14, 2020, at 4:51 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
>
> 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.
>>
>> 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.
>>
>> 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.
>>
>> This minor change claws back about 80% of the performance
>> regression.
>
> That's really useful to know. But mainly this makes me think that
> nfsd4_check_conflicting_opens() is wrong.
>
> 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.
>
> But the way I'm counting write opens probably isn't correct.
I entertained that possibility, but I couldn't figure out
a better way to count opens-for-write.
> --b.
>
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> fs/nfsd/nfs4state.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> 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]) == 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);
>> + }
>> }
>> }
--
Chuck Lever
