* [PATCH v2] nfsd: allow reaping files still under writeback
@ 2023-02-15 11:53 Jeff Layton
2023-02-19 19:45 ` Chuck Lever III
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2023-02-15 11:53 UTC (permalink / raw)
To: chuck.lever; +Cc: trond.myklebust, linux-nfs
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@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd: allow reaping files still under writeback
2023-02-15 11:53 [PATCH v2] nfsd: allow reaping files still under writeback Jeff Layton
@ 2023-02-19 19:45 ` Chuck Lever III
2023-02-19 22:58 ` Jeff Layton
2023-02-22 22:29 ` Anna Schumaker
0 siblings, 2 replies; 4+ messages in thread
From: Chuck Lever III @ 2023-02-19 19:45 UTC (permalink / raw)
To: Jeff Layton, Trond Myklebust, Anna Schumaker; +Cc: Linux NFS Mailing List
> On Feb 15, 2023, at 6:53 AM, Jeff Layton <jlayton@kernel.org> 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@kernel.org>
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.
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd: allow reaping files still under writeback
2023-02-19 19:45 ` Chuck Lever III
@ 2023-02-19 22:58 ` Jeff Layton
2023-02-22 22:29 ` Anna Schumaker
1 sibling, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2023-02-19 22:58 UTC (permalink / raw)
To: Chuck Lever III, Trond Myklebust, Anna Schumaker; +Cc: Linux NFS Mailing List
On Sun, 2023-02-19 at 19:45 +0000, Chuck Lever III wrote:
>
> > On Feb 15, 2023, at 6:53 AM, Jeff Layton <jlayton@kernel.org> 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@kernel.org>
>
> 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@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd: allow reaping files still under writeback
2023-02-19 19:45 ` Chuck Lever III
2023-02-19 22:58 ` Jeff Layton
@ 2023-02-22 22:29 ` Anna Schumaker
1 sibling, 0 replies; 4+ messages in thread
From: Anna Schumaker @ 2023-02-22 22:29 UTC (permalink / raw)
To: Chuck Lever III; +Cc: Jeff Layton, Trond Myklebust, Linux NFS Mailing List
Hi Chuck,
On Sun, Feb 19, 2023 at 2:46 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On Feb 15, 2023, at 6:53 AM, Jeff Layton <jlayton@kernel.org> 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@kernel.org>
>
> I assume that I'm taking this via the nfsd tree? If so,
> I would like an Acked-by from the NFS client maintainers...
The NFS changes look fine to me.
Acked-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> For the moment this is going to topic-filecache-cleanups,
> not to nfsd-next.
>
>
> > ---
> > 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
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-22 22:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-15 11:53 [PATCH v2] nfsd: allow reaping files still under writeback Jeff Layton
2023-02-19 19:45 ` Chuck Lever III
2023-02-19 22:58 ` Jeff Layton
2023-02-22 22:29 ` Anna Schumaker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox