diff mbox

[RFC,01/13] gfs2: Get rid of I_MUTEX_QUOTA usage

Message ID 1346878524-10585-2-git-send-email-bfields@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bruce Fields Sept. 5, 2012, 8:55 p.m. UTC
From: Jan Kara <jack@suse.cz>

GFS2 uses i_mutex on its system quota inode to synchronize writes to
quota file. Since this is an internal inode to GFS2 (not part of directory
hiearchy or visible by user) we are safe to define locking rules for it. So
let's just get it its own locking class to make it clear.

CC: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/gfs2/ops_fstype.c |    8 ++++++++
 fs/gfs2/quota.c      |    2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

J. Bruce Fields Sept. 5, 2012, 8:59 p.m. UTC | #1
On Wed, Sep 05, 2012 at 04:55:11PM -0400, J. Bruce Fields wrote:
> From: Jan Kara <jack@suse.cz>
> 
> GFS2 uses i_mutex on its system quota inode to synchronize writes to
> quota file. Since this is an internal inode to GFS2 (not part of directory
> hiearchy or visible by user) we are safe to define locking rules for it. So
> let's just get it its own locking class to make it clear.

By the way, Steve, is there any reason you haven't taken this through
the gfs2 tree?

--b.

> 
> CC: Steven Whitehouse <swhiteho@redhat.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/gfs2/ops_fstype.c |    8 ++++++++
>  fs/gfs2/quota.c      |    2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index e5af9dc..e443966 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -19,6 +19,7 @@
>  #include <linux/mount.h>
>  #include <linux/gfs2_ondisk.h>
>  #include <linux/quotaops.h>
> +#include <linux/lockdep.h>
>  
>  #include "gfs2.h"
>  #include "incore.h"
> @@ -766,6 +767,7 @@ fail:
>  	return error;
>  }
>  
> +static struct lock_class_key gfs2_quota_imutex_key;
>  
>  static int init_inodes(struct gfs2_sbd *sdp, int undo)
>  {
> @@ -803,6 +805,12 @@ static int init_inodes(struct gfs2_sbd *sdp, int undo)
>  		fs_err(sdp, "can't get quota file inode: %d\n", error);
>  		goto fail_rindex;
>  	}
> +	/*
> +	 * i_mutex on quota files is special. Since this inode is hidden system
> +	 * file, we are safe to define locking ourselves.
> +	 */
> +	lockdep_set_class(&sdp->sd_quota_inode->i_mutex,
> +			  &gfs2_quota_imutex_key);
>  
>  	error = gfs2_rindex_update(sdp);
>  	if (error)
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index a3bde91..3d9c989 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -781,7 +781,7 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
>  		return -ENOMEM;
>  
>  	sort(qda, num_qd, sizeof(struct gfs2_quota_data *), sort_qd, NULL);
> -	mutex_lock_nested(&ip->i_inode.i_mutex, I_MUTEX_QUOTA);
> +	mutex_lock(&ip->i_inode.i_mutex);
>  	for (qx = 0; qx < num_qd; qx++) {
>  		error = gfs2_glock_nq_init(qda[qx]->qd_gl, LM_ST_EXCLUSIVE,
>  					   GL_NOCACHE, &ghs[qx]);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Whitehouse Sept. 6, 2012, 2:27 p.m. UTC | #2
Hi,

On Wed, 2012-09-05 at 16:59 -0400, J. Bruce Fields wrote:
> On Wed, Sep 05, 2012 at 04:55:11PM -0400, J. Bruce Fields wrote:
> > From: Jan Kara <jack@suse.cz>
> > 
> > GFS2 uses i_mutex on its system quota inode to synchronize writes to
> > quota file. Since this is an internal inode to GFS2 (not part of directory
> > hiearchy or visible by user) we are safe to define locking rules for it. So
> > let's just get it its own locking class to make it clear.
> 
> By the way, Steve, is there any reason you haven't taken this through
> the gfs2 tree?
> 
> --b.
> 
I don't remember now, but I've pushed it into my tree for the next merge
window,

Steve.




--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Sept. 6, 2012, 5:08 p.m. UTC | #3
On Thu, Sep 06, 2012 at 03:27:46PM +0100, Steven Whitehouse wrote:
> Hi,
> 
> On Wed, 2012-09-05 at 16:59 -0400, J. Bruce Fields wrote:
> > On Wed, Sep 05, 2012 at 04:55:11PM -0400, J. Bruce Fields wrote:
> > > From: Jan Kara <jack@suse.cz>
> > > 
> > > GFS2 uses i_mutex on its system quota inode to synchronize writes to
> > > quota file. Since this is an internal inode to GFS2 (not part of directory
> > > hiearchy or visible by user) we are safe to define locking rules for it. So
> > > let's just get it its own locking class to make it clear.
> > 
> > By the way, Steve, is there any reason you haven't taken this through
> > the gfs2 tree?
> > 
> > --b.
> > 
> I don't remember now, but I've pushed it into my tree for the next merge
> window,

Thanks!

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index e5af9dc..e443966 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -19,6 +19,7 @@ 
 #include <linux/mount.h>
 #include <linux/gfs2_ondisk.h>
 #include <linux/quotaops.h>
+#include <linux/lockdep.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -766,6 +767,7 @@  fail:
 	return error;
 }
 
+static struct lock_class_key gfs2_quota_imutex_key;
 
 static int init_inodes(struct gfs2_sbd *sdp, int undo)
 {
@@ -803,6 +805,12 @@  static int init_inodes(struct gfs2_sbd *sdp, int undo)
 		fs_err(sdp, "can't get quota file inode: %d\n", error);
 		goto fail_rindex;
 	}
+	/*
+	 * i_mutex on quota files is special. Since this inode is hidden system
+	 * file, we are safe to define locking ourselves.
+	 */
+	lockdep_set_class(&sdp->sd_quota_inode->i_mutex,
+			  &gfs2_quota_imutex_key);
 
 	error = gfs2_rindex_update(sdp);
 	if (error)
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index a3bde91..3d9c989 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -781,7 +781,7 @@  static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
 		return -ENOMEM;
 
 	sort(qda, num_qd, sizeof(struct gfs2_quota_data *), sort_qd, NULL);
-	mutex_lock_nested(&ip->i_inode.i_mutex, I_MUTEX_QUOTA);
+	mutex_lock(&ip->i_inode.i_mutex);
 	for (qx = 0; qx < num_qd; qx++) {
 		error = gfs2_glock_nq_init(qda[qx]->qd_gl, LM_ST_EXCLUSIVE,
 					   GL_NOCACHE, &ghs[qx]);