From: Steven Whitehouse <swhiteho@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Russell Cattelan <cattelan@redhat.com>, cluster-devel@redhat.com
Subject: [GFS2] Fix unlink deadlocks [51/54]
Date: Mon, 05 Feb 2007 14:47:56 +0000 [thread overview]
Message-ID: <1170686876.11001.373.camel@quoit.chygwyn.com> (raw)
In-Reply-To: <1170684425.11001.267.camel@quoit.chygwyn.com>
>From 69a3a75c929f5c38cdc2dea773692aca02bbdb58 Mon Sep 17 00:00:00 2001
From: Russell Cattelan <cattelan@redhat.com>
Date: Mon, 29 Jan 2007 17:13:44 -0600
Subject: [PATCH] [GFS2] Fix unlink deadlocks
Move the glock acquisition to outside of the transactions.
Lock odering must be preserved in order to prevent ABBA
deadlocks. The current gfs2_change_nlink code would tries
to grab the glock after having started a transaction and thus is holding
the log lock. This is inconsistent with other code paths in
gfs that grab the resource group glock prior to staring
a tranactions.
One problem with this fix is that the resource group
lock is always grabbed now even if the inode still has
ref count and can not be marked for unlink.
Signed-off-by: Russell Cattelan <cattelan@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 88fcfb4..0d6831a 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -280,50 +280,6 @@ out:
return error;
}
-static int gfs2_change_nlink_i(struct gfs2_inode *ip)
-{
- struct gfs2_sbd *sdp = ip->i_inode.i_sb->s_fs_info;
- struct gfs2_inode *rindex = GFS2_I(sdp->sd_rindex);
- struct gfs2_glock *ri_gl = rindex->i_gl;
- struct gfs2_rgrpd *rgd;
- struct gfs2_holder ri_gh, rg_gh;
- int existing, error;
-
- /* if we come from rename path, we could have the lock already */
- existing = gfs2_glock_is_locked_by_me(ri_gl);
- if (!existing) {
- error = gfs2_rindex_hold(sdp, &ri_gh);
- if (error)
- goto out;
- }
-
- /* find the matching rgd */
- error = -EIO;
- rgd = gfs2_blk2rgrpd(sdp, ip->i_num.no_addr);
- if (!rgd)
- goto out_norgrp;
-
- /*
- * Eventually we may want to move rgd(s) to a linked list
- * and piggyback the free logic into one of gfs2 daemons
- * to gain some performance.
- */
- if (!rgd->rd_gl || !gfs2_glock_is_locked_by_me(rgd->rd_gl)) {
- error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, &rg_gh);
- if (error)
- goto out_norgrp;
-
- gfs2_unlink_di(&ip->i_inode); /* mark inode unlinked */
- gfs2_glock_dq_uninit(&rg_gh);
- }
-
-out_norgrp:
- if (!existing)
- gfs2_glock_dq_uninit(&ri_gh);
-out:
- return error;
-}
-
/**
* gfs2_change_nlink - Change nlink count on inode
* @ip: The GFS2 inode
@@ -365,7 +321,7 @@ int gfs2_change_nlink(struct gfs2_inode *ip, int diff)
mark_inode_dirty(&ip->i_inode);
if (ip->i_inode.i_nlink == 0)
- error = gfs2_change_nlink_i(ip);
+ gfs2_unlink_di(&ip->i_inode); /* mark inode unlinked */
return error;
}
diff --git a/fs/gfs2/ops_inode.c b/fs/gfs2/ops_inode.c
index 5591f89..f40a848 100644
--- a/fs/gfs2/ops_inode.c
+++ b/fs/gfs2/ops_inode.c
@@ -264,13 +264,23 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry)
struct gfs2_inode *dip = GFS2_I(dir);
struct gfs2_sbd *sdp = GFS2_SB(dir);
struct gfs2_inode *ip = GFS2_I(dentry->d_inode);
- struct gfs2_holder ghs[2];
+ struct gfs2_holder ghs[3];
+ struct gfs2_rgrpd *rgd;
+ struct gfs2_holder ri_gh;
int error;
+ error = gfs2_rindex_hold(sdp, &ri_gh);
+ if (error)
+ return error;
+
gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
- gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1);
+ gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1);
- error = gfs2_glock_nq_m(2, ghs);
+ rgd = gfs2_blk2rgrpd(sdp, ip->i_num.no_addr);
+ gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, ghs + 2);
+
+
+ error = gfs2_glock_nq_m(3, ghs);
if (error)
goto out;
@@ -291,10 +301,12 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry)
out_end_trans:
gfs2_trans_end(sdp);
out_gunlock:
- gfs2_glock_dq_m(2, ghs);
+ gfs2_glock_dq_m(3, ghs);
out:
gfs2_holder_uninit(ghs);
gfs2_holder_uninit(ghs + 1);
+ gfs2_holder_uninit(ghs + 2);
+ gfs2_glock_dq_uninit(&ri_gh);
return error;
}
@@ -449,13 +461,22 @@ static int gfs2_rmdir(struct inode *dir, struct dentry *dentry)
struct gfs2_inode *dip = GFS2_I(dir);
struct gfs2_sbd *sdp = GFS2_SB(dir);
struct gfs2_inode *ip = GFS2_I(dentry->d_inode);
- struct gfs2_holder ghs[2];
+ struct gfs2_holder ghs[3];
+ struct gfs2_rgrpd *rgd;
+ struct gfs2_holder ri_gh;
int error;
+
+ error = gfs2_rindex_hold(sdp, &ri_gh);
+ if (error)
+ return error;
gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1);
- error = gfs2_glock_nq_m(2, ghs);
+ rgd = gfs2_blk2rgrpd(sdp, ip->i_num.no_addr);
+ gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, ghs + 2);
+
+ error = gfs2_glock_nq_m(3, ghs);
if (error)
goto out;
@@ -483,10 +504,12 @@ static int gfs2_rmdir(struct inode *dir, struct dentry *dentry)
gfs2_trans_end(sdp);
out_gunlock:
- gfs2_glock_dq_m(2, ghs);
+ gfs2_glock_dq_m(3, ghs);
out:
gfs2_holder_uninit(ghs);
gfs2_holder_uninit(ghs + 1);
+ gfs2_holder_uninit(ghs + 2);
+ gfs2_glock_dq_uninit(&ri_gh);
return error;
}
@@ -547,7 +570,8 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
struct gfs2_inode *ip = GFS2_I(odentry->d_inode);
struct gfs2_inode *nip = NULL;
struct gfs2_sbd *sdp = GFS2_SB(odir);
- struct gfs2_holder ghs[4], r_gh;
+ struct gfs2_holder ghs[5], r_gh;
+ struct gfs2_rgrpd *nrgd;
unsigned int num_gh;
int dir_rename = 0;
int alloc_required;
@@ -587,6 +611,13 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
if (nip) {
gfs2_holder_init(nip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
num_gh++;
+ /* grab the resource lock for unlink flag twiddling
+ * this is the case of the target file already existing
+ * so we unlink before doing the rename
+ */
+ nrgd = gfs2_blk2rgrpd(sdp, nip->i_num.no_addr);
+ if (nrgd)
+ gfs2_holder_init(nrgd->rd_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh++);
}
error = gfs2_glock_nq_m(num_gh, ghs);
--
1.4.4.2
next prev parent reply other threads:[~2007-02-05 14:39 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-05 14:07 [GFS2 & DLM] Proposed patches for 2.6.20 merge window [0/54] Steven Whitehouse
2007-02-05 14:09 ` [GFS2] don't try to lockfs after shutdown [1/54] Steven Whitehouse
2007-02-05 14:09 ` [DLM] fix resend rcom lock [2/54] Steven Whitehouse
2007-02-05 14:10 ` [DLM] fix old rcom messages [3/54] Steven Whitehouse
2007-02-05 14:11 ` [DLM] add version check [4/54] Steven Whitehouse
2007-02-05 14:12 ` [DLM] fix send_args() lvb copying [5/54] Steven Whitehouse
2007-02-05 14:13 ` [DLM] fix receive_request() lvb copying [6/54] Steven Whitehouse
2007-02-05 14:14 ` [DLM] fix lost flags in stub replies Steven Whitehouse
2007-02-05 14:15 ` [DLM] fs/dlm/lowcomms-tcp.c: remove 2 functions [8/54] Steven Whitehouse
2007-02-05 14:16 ` [GFS2] Fix DIO deadlock [9/54] Steven Whitehouse
2007-02-05 14:17 ` [GFS2] Fail over to readpage for stuffed files [10/54] Steven Whitehouse
2007-02-05 14:18 ` [GFS2] Fix change nlink deadlock [11/54] Steven Whitehouse
2007-02-05 14:19 ` [DLM] Fix schedule() calls [12/54] Steven Whitehouse
2007-02-05 14:19 ` [DLM] Fix spin lock already unlocked bug [13/54] Steven Whitehouse
2007-02-05 14:20 ` [GFS2] Fix ordering of page disposal vs. glock_dq [14/54] Steven Whitehouse
2007-02-05 14:21 ` [GFS2] BZ 217008 fsfuzzer fix [15/54] Steven Whitehouse
2007-02-05 14:22 ` [GFS2] Fix gfs2_rename deadlock [16/54] Steven Whitehouse
2007-02-05 14:22 ` [DLM] change some log_error to log_debug [17/54] Steven Whitehouse
2007-02-05 14:23 ` [DLM] rename dlm_config_info fields [18/54] Steven Whitehouse
2007-02-05 14:24 ` [DLM] add config entry to enable log_debug [16/54] Steven Whitehouse
2007-02-05 14:25 ` [DLM] expose dlm_config_info fields in configfs [20/54] Steven Whitehouse
2007-02-05 14:26 ` [GFS2] gfs2 knows of directories which it chooses not to display [21/54] Steven Whitehouse
2007-02-05 14:27 ` [GFS2] make gfs2_change_nlink_i() static [22/54] Steven Whitehouse
2007-02-05 14:28 ` [DLM] Use workqueues for dlm lowcomms [23/54] Steven Whitehouse
2007-02-05 14:29 ` [DLM] fix user unlocking [24/54] Steven Whitehouse
2007-02-05 14:29 ` [DLM] fix master recovery [25/54] Steven Whitehouse
2007-02-05 14:30 ` [GFS2] Add writepages for "data=writeback" mounts [26/54] Steven Whitehouse
2007-02-05 14:31 ` [GFS2] Clean up/speed up readdir [27/54] Steven Whitehouse
2007-02-05 14:31 ` [GFS2] Remove max_atomic_write tunable [28/54] Steven Whitehouse
2007-02-05 14:32 ` [GFS2] Shrink gfs2_inode memory by half [29/54] Steven Whitehouse
2007-02-05 14:33 ` [GFS2] Remove the "greedy" function from glock.[ch] [30/54] Steven Whitehouse
2007-02-05 14:34 ` [GFS2] Remove unused go_callback operation [31/54] Steven Whitehouse
2007-02-05 14:34 ` [GFS2] Remove local exclusive glock mode [32/54] Steven Whitehouse
2007-02-05 14:35 ` [DLM] lowcomms tidy [33/54] Steven Whitehouse
2007-02-05 14:35 ` [GFS2] Tidy up glops calls [34/54] Steven Whitehouse
2007-02-05 14:36 ` [DLM] fix lowcomms receiving [35/54] Steven Whitehouse
2007-02-05 14:37 ` [GFS2] Remove queue_empty() function [36/54] Steven Whitehouse
2007-02-05 14:37 ` [GFS2] Compile fix for glock.c [37/54] Steven Whitehouse
2007-02-05 14:38 ` [GFS2] use CURRENT_TIME_SEC instead of get_seconds in gfs2 [38/54] Steven Whitehouse
2007-02-05 14:39 ` [GFS2] Fix typo in glock.c [39/54] Steven Whitehouse
2007-02-05 14:40 ` [DLM] Make sock_sem into a mutex [40/54] Steven Whitehouse
2007-02-05 14:40 ` [DLM] saved dlm message can be dropped [41/54] Steven Whitehouse
2007-02-05 14:41 ` [DLM] can miss clearing resend flag Steven Whitehouse
2007-02-05 14:41 ` [GFS2] Fix recursive locking attempt with NFS [43/54] Steven Whitehouse
2007-02-05 14:42 ` [GFS2] Fix list corruption in lops.c [44/54] Steven Whitehouse
2007-02-05 14:43 ` [GFS2] increase default lock limit [45/54] Steven Whitehouse
2007-02-05 14:44 ` [GFS2] make lock_dlm drop_count tunable in sysfs [46/54] Steven Whitehouse
2007-02-05 14:44 ` [GFS2/DLM] use sysfs Steven Whitehouse
2007-02-05 14:45 ` [GFS2/DLM] fix GFS2 circular dependency [48/54] Steven Whitehouse
2007-02-05 14:46 ` [GFS2] more CURRENT_TIME_SEC [49/54] Steven Whitehouse
2007-02-05 14:47 ` [GFS2] Put back semaphore to avoid umount proble Steven Whitehouse
2007-02-05 14:47 ` Steven Whitehouse [this message]
2007-02-05 14:48 ` [DLM/GFS2] indent help text [52/54] Steven Whitehouse
2007-02-05 14:49 ` [DLM] zero new user lvbs [53/54] Steven Whitehouse
2007-02-05 14:50 ` [DLM] fix softlockup in dlm_recv [54/54] Steven Whitehouse
2007-02-07 13:20 ` [GFS2 & DLM] Pull request Steven Whitehouse
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=1170686876.11001.373.camel@quoit.chygwyn.com \
--to=swhiteho@redhat.com \
--cc=cattelan@redhat.com \
--cc=cluster-devel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
/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
all inboxes | Powered by JetHome®