On Sun, 2023-02-19 at 19:45 +0000, Chuck Lever III wrote:
>
> > On Feb 15, 2023, at 6:53 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >
> > On most filesystems, there is 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.
> >
> > The main exception is NFS, which flushes all of its dirty data on last
> > close. Add a new EXPORT_OP_FLUSH_ON_CLOSE flag to allow filesystems to
> > signal that they do this, and only skip closing files under writeback on
> > such filesystems.
> >
> > Also, remove a redundant NULL file pointer check in
> > nfsd_file_check_writeback, and clean up nfs's export op flag
> > definitions.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>
> I assume that I'm taking this via the nfsd tree? If so,
> I would like an Acked-by from the NFS client maintainers...
>
> For the moment this is going to topic-filecache-cleanups,
> not to nfsd-next.
>
>
No need to rush on this one, IMO. It's not super-critical or anything.
The topic branch is probably fine.
> > ---
> > fs/nfs/export.c | 9 ++++++---
> > fs/nfsd/filecache.c | 11 ++++++++++-
> > include/linux/exportfs.h | 1 +
> > 3 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> > index 0a5ee1754d50..102a454e27c9 100644
> > --- a/fs/nfs/export.c
> > +++ b/fs/nfs/export.c
> > @@ -156,7 +156,10 @@ const struct export_operations nfs_export_ops = {
> > .fh_to_dentry = nfs_fh_to_dentry,
> > .get_parent = nfs_get_parent,
> > .fetch_iversion = nfs_fetch_iversion,
> > - .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK|
> > - EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS|
> > - EXPORT_OP_NOATOMIC_ATTR,
> > + .flags = EXPORT_OP_NOWCC |
> > + EXPORT_OP_NOSUBTREECHK |
> > + EXPORT_OP_CLOSE_BEFORE_UNLINK |
> > + EXPORT_OP_REMOTE_FS |
> > + EXPORT_OP_NOATOMIC_ATTR |
> > + EXPORT_OP_FLUSH_ON_CLOSE,
> > };
> > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> > index e6617431df7c..98e1ab013ac0 100644
> > --- a/fs/nfsd/filecache.c
> > +++ b/fs/nfsd/filecache.c
> > @@ -302,8 +302,17 @@ 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))
> > + /* File not open for write? */
> > + if (!(file->f_mode & FMODE_WRITE))
> > return false;
> > +
> > + /*
> > + * Some filesystems (e.g. NFS) flush all dirty data on close.
> > + * On others, there is no need to wait for writeback.
> > + */
> > + if (!(file_inode(file)->i_sb->s_export_op->flags & EXPORT_OP_FLUSH_ON_CLOSE))
> > + return false;
> > +
> > mapping = file->f_mapping;
> > return mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
> > mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
> > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > index fe848901fcc3..218fc5c54e90 100644
> > --- a/include/linux/exportfs.h
> > +++ b/include/linux/exportfs.h
> > @@ -221,6 +221,7 @@ struct export_operations {
> > #define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply
> > atomic attribute updates
> > */
> > +#define EXPORT_OP_FLUSH_ON_CLOSE (0x20) /* fs flushes file data on close */
> > unsigned long flags;
> > };
> >
> > --
> > 2.39.1
> >
>
> --
> Chuck Lever
>
>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
