orangefs: implement statx

Commit Message

Martin Brandenburg March 28, 2017, 7:49 p.m. UTC

Fortunately OrangeFS has had a getattr request mask for a long time.

The server basically has two difficulty levels for attributes.  Fetching
any attribute except size requires communicating with the metadata
server for that handle.  Since all the attributes are right there, it
makes sense to return them all.  Fetching the size requires
communicating with every I/O server (that the file is distributed
across).  Therefore if asked for anything except size, get everything
except size, and if asked for size, get everything.

Signed-off-by: Martin Brandenburg <[e-mail]>
---
 fs/orangefs/file.c            |  6 ++++--
 fs/orangefs/inode.c           | 15 ++++++++++-----
 fs/orangefs/orangefs-kernel.h |  3 ++-
 fs/orangefs/orangefs-utils.c  | 13 +++++++++++--
 4 files changed, 27 insertions(+), 10 deletions(-)

Comments

Martin Brandenburg March 29, 2017, 3:41 p.m. UTC | #1

The last version had a problem with xfstests generic/130, generic/169,
generic/246, generic/309, and generic/393.

Other code, namely orangefs_setattr_size (truncate), looks at i_size
without knowledge that it was never set.  This doesn't make decisions
whether to actually do the truncate based on the possibly not set (or
stale!) i_size.

I'm still not entirely happy with it though.  I feel like we may be
missing other code which looks at i_size.

Martin Brandenburg (2):
  orangefs: implement statx
  orangefs: do not check possibly stale size on truncate

 fs/orangefs/file.c            |  6 +++--
 fs/orangefs/inode.c           | 18 ++++++++-----
 fs/orangefs/namei.c           |  3 +++
 fs/orangefs/orangefs-kernel.h |  4 ++-
 fs/orangefs/orangefs-utils.c  | 63 +++++++++++++++++++++++++++++--------------
 5 files changed, 64 insertions(+), 30 deletions(-)

Patch

diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index e6bbc80..b421df1 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -475,7 +475,8 @@  static ssize_t orangefs_file_write_iter(struct kiocb *iocb, struct iov_iter *ite
 
 	/* Make sure generic_write_checks sees an up to date inode size. */
 	if (file->f_flags & O_APPEND) {
-		rc = orangefs_inode_getattr(file->f_mapping->host, 0, 1);
+		rc = orangefs_inode_getattr(file->f_mapping->host, 0, 1,
+		    STATX_SIZE);
 		if (rc == -ESTALE)
 			rc = -EIO;
 		if (rc) {
@@ -693,7 +694,8 @@  static loff_t orangefs_file_llseek(struct file *file, loff_t offset, int origin)
 		 * NOTE: We are only interested in file size here,
 		 * so we set mask accordingly.
 		 */
-		ret = orangefs_inode_getattr(file->f_mapping->host, 0, 1);
+		ret = orangefs_inode_getattr(file->f_mapping->host, 0, 1,
+		    STATX_SIZE);
 		if (ret == -ESTALE)
 			ret = -EIO;
 		if (ret) {
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index a304bf3..ee28364 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -161,7 +161,7 @@  static int orangefs_setattr_size(struct inode *inode, struct iattr *iattr)
 		     iattr->ia_size);
 
 	/* Ensure that we have a up to date size, so we know if it changed. */
-	ret = orangefs_inode_getattr(inode, 0, 1);
+	ret = orangefs_inode_getattr(inode, 0, 1, STATX_SIZE);
 	if (ret == -ESTALE)
 		ret = -EIO;
 	if (ret) {
@@ -256,13 +256,18 @@  int orangefs_getattr(const struct path *path, struct kstat *stat,
 		     "orangefs_getattr: called on %pd\n",
 		     path->dentry);
 
-	ret = orangefs_inode_getattr(inode, 0, 0);
+	ret = orangefs_inode_getattr(inode, 0, 0, request_mask);
 	if (ret == 0) {
 		generic_fillattr(inode, stat);
 
 		/* override block size reported to stat */
 		orangefs_inode = ORANGEFS_I(inode);
 		stat->blksize = orangefs_inode->blksize;
+
+		if (request_mask & STATX_SIZE)
+			stat->result_mask = STATX_BASIC_STATS;
+		else
+			stat->result_mask = STATX_BASIC_STATS & ~STATX_SIZE;
 	}
 	return ret;
 }
@@ -277,7 +282,7 @@  int orangefs_permission(struct inode *inode, int mask)
 	gossip_debug(GOSSIP_INODE_DEBUG, "%s: refreshing\n", __func__);
 
 	/* Make sure the permission (and other common attrs) are up to date. */
-	ret = orangefs_inode_getattr(inode, 0, 0);
+	ret = orangefs_inode_getattr(inode, 0, 0, STATX_MODE);
 	if (ret < 0)
 		return ret;
 
@@ -375,7 +380,7 @@  struct inode *orangefs_iget(struct super_block *sb, struct orangefs_object_kref
 	if (!inode || !(inode->i_state & I_NEW))
 		return inode;
 
-	error = orangefs_inode_getattr(inode, 1, 1);
+	error = orangefs_inode_getattr(inode, 1, 1, STATX_ALL);
 	if (error) {
 		iget_failed(inode);
 		return ERR_PTR(error);
@@ -420,7 +425,7 @@  struct inode *orangefs_new_inode(struct super_block *sb, struct inode *dir,
 	orangefs_set_inode(inode, ref);
 	inode->i_ino = hash;	/* needed for stat etc */
 
-	error = orangefs_inode_getattr(inode, 1, 1);
+	error = orangefs_inode_getattr(inode, 1, 1, STATX_ALL);
 	if (error)
 		goto out_iput;
 
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 5e48a0b..016d988 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -499,7 +499,8 @@  int orangefs_inode_setxattr(struct inode *inode,
 			 size_t size,
 			 int flags);
 
-int orangefs_inode_getattr(struct inode *inode, int new, int bypass);
+int orangefs_inode_getattr(struct inode *inode, int new, int bypass,
+    u32 request_mask);
 
 int orangefs_inode_check_changed(struct inode *inode);
 
diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c
index 9b96b99..e6e01f6 100644
--- a/fs/orangefs/orangefs-utils.c
+++ b/fs/orangefs/orangefs-utils.c
@@ -251,7 +251,8 @@  static int orangefs_inode_is_stale(struct inode *inode, int new,
 	return 0;
 }
 
-int orangefs_inode_getattr(struct inode *inode, int new, int bypass)
+int orangefs_inode_getattr(struct inode *inode, int new, int bypass,
+    u32 request_mask)
 {
 	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
 	struct orangefs_kernel_op_s *new_op;
@@ -270,7 +271,15 @@  int orangefs_inode_getattr(struct inode *inode, int new, int bypass)
 	if (!new_op)
 		return -ENOMEM;
 	new_op->upcall.req.getattr.refn = orangefs_inode->refn;
-	new_op->upcall.req.getattr.mask = ORANGEFS_ATTR_SYS_ALL_NOHINT;
+	/*
+	 * Size is the hardest attribute to get.  The incremental cost of any
+	 * other attribute is essentially free.
+	 */
+	if (request_mask & STATX_SIZE)
+		new_op->upcall.req.getattr.mask = ORANGEFS_ATTR_SYS_ALL_NOHINT;
+	else
+		new_op->upcall.req.getattr.mask =
+		    ORANGEFS_ATTR_SYS_ALL_NOHINT & ~ORANGEFS_ATTR_SYS_SIZE;
 
 	ret = service_operation(new_op, __func__,
 	    get_interruptible_flag(inode));