public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: xfs@oss.sgi.com
Subject: [PATCH 1/5 v3] xfs: always take the iolock around xfs_setattr_size
Date: Mon, 14 Oct 2013 07:09:35 -0700	[thread overview]
Message-ID: <20131014140935.GA32035@infradead.org> (raw)
In-Reply-To: <20131012075639.940898263@bombadil.infradead.org>

There is no reason to conditionally take the iolock inside xfs_setattr_size
when we can let the caller handle it unconditionally, which just incrases
the lock hold time for the case where it was previously taken internally
by a few instructions.

Signed-off-by: Christoph Hellwig <hch@lst.de>

---
 fs/xfs/xfs_bmap_util.c |    3 +--
 fs/xfs/xfs_file.c      |    2 +-
 fs/xfs/xfs_iops.c      |   39 +++++++++++++++++++++------------------
 fs/xfs/xfs_iops.h      |    2 +-
 4 files changed, 24 insertions(+), 22 deletions(-)

Index: xfs/fs/xfs/xfs_file.c
===================================================================
--- xfs.orig/fs/xfs/xfs_file.c	2013-10-11 20:05:38.272512388 +0200
+++ xfs/fs/xfs/xfs_file.c	2013-10-14 16:07:38.316153285 +0200
@@ -852,7 +852,7 @@ xfs_file_fallocate(
 
 		iattr.ia_valid = ATTR_SIZE;
 		iattr.ia_size = new_size;
-		error = -xfs_setattr_size(ip, &iattr, XFS_ATTR_NOLOCK);
+		error = -xfs_setattr_size(ip, &iattr);
 	}
 
 out_unlock:
Index: xfs/fs/xfs/xfs_iops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iops.c	2013-10-11 20:05:38.272512388 +0200
+++ xfs/fs/xfs/xfs_iops.c	2013-10-14 15:28:38.900097685 +0200
@@ -709,8 +709,7 @@ out_dqrele:
 int
 xfs_setattr_size(
 	struct xfs_inode	*ip,
-	struct iattr		*iattr,
-	int			flags)
+	struct iattr		*iattr)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct inode		*inode = VFS_I(ip);
@@ -733,15 +732,11 @@ xfs_setattr_size(
 	if (error)
 		return XFS_ERROR(error);
 
+	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
 	ASSERT(S_ISREG(ip->i_d.di_mode));
 	ASSERT((mask & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
 			ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
 
-	if (!(flags & XFS_ATTR_NOLOCK)) {
-		lock_flags |= XFS_IOLOCK_EXCL;
-		xfs_ilock(ip, lock_flags);
-	}
-
 	oldsize = inode->i_size;
 	newsize = iattr->ia_size;
 
@@ -750,12 +745,11 @@ xfs_setattr_size(
 	 */
 	if (newsize == 0 && oldsize == 0 && ip->i_d.di_nextents == 0) {
 		if (!(mask & (ATTR_CTIME|ATTR_MTIME)))
-			goto out_unlock;
+			return 0;
 
 		/*
 		 * Use the regular setattr path to update the timestamps.
 		 */
-		xfs_iunlock(ip, lock_flags);
 		iattr->ia_valid &= ~ATTR_SIZE;
 		return xfs_setattr_nonsize(ip, iattr, 0);
 	}
@@ -765,7 +759,7 @@ xfs_setattr_size(
 	 */
 	error = xfs_qm_dqattach(ip, 0);
 	if (error)
-		goto out_unlock;
+		return error;
 
 	/*
 	 * Now we can make the changes.  Before we join the inode to the
@@ -783,7 +777,7 @@ xfs_setattr_size(
 		 */
 		error = xfs_zero_eof(ip, newsize, oldsize);
 		if (error)
-			goto out_unlock;
+			return error;
 	}
 
 	/*
@@ -802,7 +796,7 @@ xfs_setattr_size(
 		error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
 						      ip->i_d.di_size, newsize);
 		if (error)
-			goto out_unlock;
+			return error;
 	}
 
 	/*
@@ -812,7 +806,7 @@ xfs_setattr_size(
 
 	error = -block_truncate_page(inode->i_mapping, newsize, xfs_get_blocks);
 	if (error)
-		goto out_unlock;
+		return error;
 
 	tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_SIZE);
 	error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
@@ -916,12 +910,21 @@ out_trans_cancel:
 
 STATIC int
 xfs_vn_setattr(
-	struct dentry	*dentry,
-	struct iattr	*iattr)
+	struct dentry		*dentry,
+	struct iattr		*iattr)
 {
-	if (iattr->ia_valid & ATTR_SIZE)
-		return -xfs_setattr_size(XFS_I(dentry->d_inode), iattr, 0);
-	return -xfs_setattr_nonsize(XFS_I(dentry->d_inode), iattr, 0);
+	struct xfs_inode	*ip = XFS_I(dentry->d_inode);
+	int			error;
+
+	if (iattr->ia_valid & ATTR_SIZE) {
+		xfs_ilock(ip, XFS_IOLOCK_EXCL);
+		error = xfs_setattr_size(ip, iattr);
+		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+	} else {
+		error = xfs_setattr_nonsize(ip, iattr, 0);
+	}
+
+	return -error;
 }
 
 STATIC int
Index: xfs/fs/xfs/xfs_bmap_util.c
===================================================================
--- xfs.orig/fs/xfs/xfs_bmap_util.c	2013-10-11 20:05:38.272512388 +0200
+++ xfs/fs/xfs/xfs_bmap_util.c	2013-10-14 16:07:38.316153285 +0200
@@ -1622,8 +1622,7 @@ xfs_change_file_space(
 		iattr.ia_valid = ATTR_SIZE;
 		iattr.ia_size = startoffset;
 
-		error = xfs_setattr_size(ip, &iattr,
-					 attr_flags | XFS_ATTR_NOLOCK);
+		error = xfs_setattr_size(ip, &iattr);
 		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 
 		if (error)
Index: xfs/fs/xfs/xfs_iops.h
===================================================================
--- xfs.orig/fs/xfs/xfs_iops.h	2013-10-11 20:05:38.272512388 +0200
+++ xfs/fs/xfs/xfs_iops.h	2013-10-14 16:07:38.440153288 +0200
@@ -38,6 +38,6 @@ extern void xfs_setup_inode(struct xfs_i
 
 extern int xfs_setattr_nonsize(struct xfs_inode *ip, struct iattr *vap,
 			       int flags);
-extern int xfs_setattr_size(struct xfs_inode *ip, struct iattr *vap, int flags);
+extern int xfs_setattr_size(struct xfs_inode *ip, struct iattr *vap);
 
 #endif /* __XFS_IOPS_H__ */

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2013-10-14 14:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-12  7:55 [PATCH 0/5] refactor the preallocation and hole punching code Christoph Hellwig
2013-10-12  7:55 ` [PATCH 1/5] xfs: always take the iolock around xfs_setattr_size Christoph Hellwig
2013-10-14  4:50   ` Dave Chinner
2013-10-14  7:29     ` Christoph Hellwig
2013-10-14 14:09   ` Christoph Hellwig [this message]
2013-10-14 20:28     ` [PATCH 1/5 v3] " Dave Chinner
2013-10-12  7:55 ` [PATCH 2/5] xfs: remove the unused XFS_ATTR_NONBLOCK flag Christoph Hellwig
2013-10-14  4:51   ` Dave Chinner
2013-10-17 20:01     ` Ben Myers
2013-10-17 20:03       ` Christoph Hellwig
2013-10-17 21:17         ` Ben Myers
2013-10-12  7:55 ` [PATCH 3/5] xfs: always hold the iolock when calling xfs_change_file_space Christoph Hellwig
2013-10-14  4:55   ` Dave Chinner
2013-10-12  7:55 ` [PATCH 4/5] xfs: simplify the fallocate path Christoph Hellwig
2013-10-14  5:04   ` Dave Chinner
2013-10-14  7:30     ` Christoph Hellwig
2013-10-14 20:03       ` Dave Chinner
2013-10-12  7:55 ` [PATCH 5/5] xfs: fold xfs_change_file_space into xfs_ioc_space Christoph Hellwig
2013-10-14  5:08   ` Dave Chinner
2013-10-15 15:31     ` Christoph Hellwig
2013-10-15 21:47       ` Dave Chinner
2013-10-16  7:03         ` Christoph Hellwig
2013-10-21 21:58 ` [PATCH 0/5] refactor the preallocation and hole punching code Ben Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131014140935.GA32035@infradead.org \
    --to=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox