Re: [PATCH] nfsd: allow reaping files that are still under writeback

[Chuck Lever III <chuck.lever@oracle.com>]

> On Feb 14, 2023, at 10:00 AM, Trond Myklebust <trondmy@kernel.org> wrote:
> 
> On Tue, 2023-02-14 at 14:48 +0000, Chuck Lever III wrote:
>> 
>> 
>>> On Feb 13, 2023, at 3:23 PM, Jeff Layton <jlayton@kernel.org>
>>> wrote:
>>> 
>>> There's no reason to delay reaping an nfsd_file just because its
>>> underlying inode is still under writeback. nfsd just relies on
>>> client
>>> activity or the local flusher threads to do writeback.
>>> 
>>> Holding the file open does nothing to facilitate that, nor does it
>>> help
>>> with tracking errors. Just allow it to close and let the kernel do
>>> writeback as it normally would.
>>> 
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> 
>> Thanks! Applied to topic-filecache-cleanups.
>> 
>> 
>>> ---
>>> fs/nfsd/filecache.c | 22 ----------------------
>>> 1 file changed, 22 deletions(-)
>>> 
>>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
>>> index e6617431df7c..3b9a10378c83 100644
>>> --- a/fs/nfsd/filecache.c
>>> +++ b/fs/nfsd/filecache.c
>>> @@ -296,19 +296,6 @@ nfsd_file_free(struct nfsd_file *nf)
>>>         call_rcu(&nf->nf_rcu, nfsd_file_slab_free);
>>> }
>>> 
>>> -static bool
>>> -nfsd_file_check_writeback(struct nfsd_file *nf)
>>> -{
>>> -       struct file *file = nf->nf_file;
>>> -       struct address_space *mapping;
>>> -
>>> -       if (!file || !(file->f_mode & FMODE_WRITE))
>>> -               return false;
>>> -       mapping = file->f_mapping;
>>> -       return mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
>>> -               mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
>>> -}
>>> -
>>> static bool nfsd_file_lru_add(struct nfsd_file *nf)
>>> {
>>>         set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
>>> @@ -438,15 +425,6 @@ nfsd_file_lru_cb(struct list_head *item,
>>> struct list_lru_one *lru,
>>>         /* We should only be dealing with GC entries here */
>>>         WARN_ON_ONCE(!test_bit(NFSD_FILE_GC, &nf->nf_flags));
>>> 
>>> -       /*
>>> -        * Don't throw out files that are still undergoing I/O or
>>> -        * that have uncleared errors pending.
>>> -        */
>>> -       if (nfsd_file_check_writeback(nf)) {
>>> -               trace_nfsd_file_gc_writeback(nf);
>>> -               return LRU_SKIP;
>>> -       }
>>> -
>>>         /* If it was recently added to the list, skip it */
>>>         if (test_and_clear_bit(NFSD_FILE_REFERENCED, &nf-
>>>> nf_flags)) {
>>>                 trace_nfsd_file_gc_referenced(nf);
>>> -- 
>>> 2.39.1
>>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
> 
> Wait... There is a good reason for wanting to do this in the case of
> NFS re-exports, since close() is a very expensive operation if the file
> has dirty data.

Then perhaps skipping these files can be gated on an EXPORT_OP flag?


--
Chuck Lever