diff mbox series

[3/4] nfs: don't mangle i_version on NFS

Message ID 1605583086-19869-3-git-send-email-bfields@redhat.com (mailing list archive)
State New, archived
Headers show
Series [1/4] nfsd: move fill_{pre,post}_wcc to nfsfh.c | expand

Commit Message

Bruce Fields Nov. 17, 2020, 3:18 a.m. UTC 
From: "J. Bruce Fields" <bfields@redhat.com>

The i_version on NFS has pretty much opaque to the client, so we don't
want to give the low bit any special interpretation.

Define a new FS_PRIVATE_I_VERSION flag for filesystems that manage the
i_version on their own.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/export.c          | 1 +
 include/linux/exportfs.h | 1 +
 include/linux/iversion.h | 4 ++++
 3 files changed, 6 insertions(+)

Comments

Jeffrey Layton Nov. 17, 2020, 12:27 p.m. UTC | #1 
On Mon, 2020-11-16 at 22:18 -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> The i_version on NFS has pretty much opaque to the client, so we don't
> want to give the low bit any special interpretation.
> 
> Define a new FS_PRIVATE_I_VERSION flag for filesystems that manage the
> i_version on their own.
> 

Description here doesn't quite match the patch...

> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfs/export.c          | 1 +
>  include/linux/exportfs.h | 1 +
>  include/linux/iversion.h | 4 ++++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> index 3430d6891e89..c2eb915a54ca 100644
> --- a/fs/nfs/export.c
> +++ b/fs/nfs/export.c
> @@ -171,4 +171,5 @@ const struct export_operations nfs_export_ops = {
>  	.encode_fh = nfs_encode_fh,
>  	.fh_to_dentry = nfs_fh_to_dentry,
>  	.get_parent = nfs_get_parent,
> +	.fetch_iversion = inode_peek_iversion_raw,
>  };
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 3ceb72b67a7a..6000121a201f 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -213,6 +213,7 @@ struct export_operations {
>  			  bool write, u32 *device_generation);
>  	int (*commit_blocks)(struct inode *inode, struct iomap *iomaps,
>  			     int nr_iomaps, struct iattr *iattr);
> +	u64 (*fetch_iversion)(const struct inode *);
>  };
>  
> 
> 
> 
> 
> 
> 
> 
>  extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
> diff --git a/include/linux/iversion.h b/include/linux/iversion.h
> index 2917ef990d43..481b3debf6bb 100644
> --- a/include/linux/iversion.h
> +++ b/include/linux/iversion.h
> @@ -3,6 +3,7 @@
>  #define _LINUX_IVERSION_H
>  
> 
> 
> 
> 
> 
> 
> 
>  #include <linux/fs.h>
> +#include <linux/exportfs.h>
>  
> 
> 
> 
> 
> 
> 
> 
>  /*
>   * The inode->i_version field:
> @@ -306,6 +307,9 @@ inode_query_iversion(struct inode *inode)
>  {
>  	u64 cur, old, new;
>  
> 
> 
> 
> 
> 
> 
> 
> +	if (inode->i_sb->s_export_op->fetch_iversion)
> +		return inode->i_sb->s_export_op->fetch_iversion(inode);
> +

This looks dangerous -- s_export_op could be a NULL pointer.

>  	cur = inode_peek_iversion_raw(inode);
>  	for (;;) {
>  		/* If flag is already set, then no need to swap */
J. Bruce Fields Nov. 17, 2020, 2:14 p.m. UTC | #2 
On Tue, Nov 17, 2020 at 07:27:03AM -0500, Jeff Layton wrote:
> On Mon, 2020-11-16 at 22:18 -0500, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > The i_version on NFS has pretty much opaque to the client, so we don't
> > want to give the low bit any special interpretation.
> > 
> > Define a new FS_PRIVATE_I_VERSION flag for filesystems that manage the
> > i_version on their own.
> > 
> 
> Description here doesn't quite match the patch...

Oops, thanks.--b.

> 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  fs/nfs/export.c          | 1 +
> >  include/linux/exportfs.h | 1 +
> >  include/linux/iversion.h | 4 ++++
> >  3 files changed, 6 insertions(+)
> > 
> > diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> > index 3430d6891e89..c2eb915a54ca 100644
> > --- a/fs/nfs/export.c
> > +++ b/fs/nfs/export.c
> > @@ -171,4 +171,5 @@ const struct export_operations nfs_export_ops = {
> >  	.encode_fh = nfs_encode_fh,
> >  	.fh_to_dentry = nfs_fh_to_dentry,
> >  	.get_parent = nfs_get_parent,
> > +	.fetch_iversion = inode_peek_iversion_raw,
> >  };
> > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > index 3ceb72b67a7a..6000121a201f 100644
> > --- a/include/linux/exportfs.h
> > +++ b/include/linux/exportfs.h
> > @@ -213,6 +213,7 @@ struct export_operations {
> >  			  bool write, u32 *device_generation);
> >  	int (*commit_blocks)(struct inode *inode, struct iomap *iomaps,
> >  			     int nr_iomaps, struct iattr *iattr);
> > +	u64 (*fetch_iversion)(const struct inode *);
> >  };
> >  
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >  extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
> > diff --git a/include/linux/iversion.h b/include/linux/iversion.h
> > index 2917ef990d43..481b3debf6bb 100644
> > --- a/include/linux/iversion.h
> > +++ b/include/linux/iversion.h
> > @@ -3,6 +3,7 @@
> >  #define _LINUX_IVERSION_H
> >  
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >  #include <linux/fs.h>
> > +#include <linux/exportfs.h>
> >  
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> >  /*
> >   * The inode->i_version field:
> > @@ -306,6 +307,9 @@ inode_query_iversion(struct inode *inode)
> >  {
> >  	u64 cur, old, new;
> >  
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > +	if (inode->i_sb->s_export_op->fetch_iversion)
> > +		return inode->i_sb->s_export_op->fetch_iversion(inode);
> > +
> 
> This looks dangerous -- s_export_op could be a NULL pointer.
> 
> >  	cur = inode_peek_iversion_raw(inode);
> >  	for (;;) {
> >  		/* If flag is already set, then no need to swap */
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
diff mbox series

Patch

diff --git a/fs/nfs/export.c b/fs/nfs/export.c
index 3430d6891e89..c2eb915a54ca 100644
--- a/fs/nfs/export.c
+++ b/fs/nfs/export.c
@@ -171,4 +171,5 @@  const struct export_operations nfs_export_ops = {
 	.encode_fh = nfs_encode_fh,
 	.fh_to_dentry = nfs_fh_to_dentry,
 	.get_parent = nfs_get_parent,
+	.fetch_iversion = inode_peek_iversion_raw,
 };
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 3ceb72b67a7a..6000121a201f 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -213,6 +213,7 @@  struct export_operations {
 			  bool write, u32 *device_generation);
 	int (*commit_blocks)(struct inode *inode, struct iomap *iomaps,
 			     int nr_iomaps, struct iattr *iattr);
+	u64 (*fetch_iversion)(const struct inode *);
 };
 
 extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
diff --git a/include/linux/iversion.h b/include/linux/iversion.h
index 2917ef990d43..481b3debf6bb 100644
--- a/include/linux/iversion.h
+++ b/include/linux/iversion.h
@@ -3,6 +3,7 @@ 
 #define _LINUX_IVERSION_H
 
 #include <linux/fs.h>
+#include <linux/exportfs.h>
 
 /*
  * The inode->i_version field:
@@ -306,6 +307,9 @@  inode_query_iversion(struct inode *inode)
 {
 	u64 cur, old, new;
 
+	if (inode->i_sb->s_export_op->fetch_iversion)
+		return inode->i_sb->s_export_op->fetch_iversion(inode);
+
 	cur = inode_peek_iversion_raw(inode);
 	for (;;) {
 		/* If flag is already set, then no need to swap */