On Tue, 2023-02-14 at 14:48 +0000, Chuck Lever III wrote:
>
>
> > On Feb 13, 2023, at 3:23 PM, Jeff Layton <jlayton@xxxxxxxxxx>
> > 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@xxxxxxxxxx>
>
> 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.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
