From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Fri, 20 Nov 2015 13:33:22 +0000 Subject: [Cluster-devel] [GFS2 PATCH 1/2] GFS2: Make gfs2_clear_inode() queue the final put In-Reply-To: <1447958561-2584-2-git-send-email-rpeterso@redhat.com> References: <1447958561-2584-1-git-send-email-rpeterso@redhat.com> <1447958561-2584-2-git-send-email-rpeterso@redhat.com> Message-ID: <564F2122.1040506@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, On 19/11/15 18:42, Bob Peterson wrote: > This patch changes function gfs2_clear_inode() so that instead > of calling gfs2_glock_put directly() most of the time, it queues > the glock to the delayed work queue. That avoids a possible > deadlock where it calls dlm during a fence operation: > dlm waits for a fence operation, the fence operation waits for > memory, the shrinker waits for gfs2 to free an inode from memory, > but gfs2 waits for dlm. > > Signed-off-by: Bob Peterson > --- > fs/gfs2/glock.c | 34 +++++++++++++++++----------------- > fs/gfs2/glock.h | 1 + > fs/gfs2/super.c | 5 ++++- > 3 files changed, 22 insertions(+), 18 deletions(-) [snip] Most of the patch seems to just rename the workqueue which makes it tricky to spot the other changes. However, the below code seems to be the new bit.. > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c > index 9d5c3f7..46e5004 100644 > --- a/fs/gfs2/super.c > +++ b/fs/gfs2/super.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1614,7 +1615,9 @@ out: > ip->i_gl->gl_object = NULL; > flush_delayed_work(&ip->i_gl->gl_work); > gfs2_glock_add_to_lru(ip->i_gl); > - gfs2_glock_put(ip->i_gl); > + if (queue_delayed_work(gfs2_glock_workqueue, > + &ip->i_gl->gl_work, 0) == 0) > + gfs2_glock_put(ip->i_gl); > ip->i_gl = NULL; > if (ip->i_iopen_gh.gh_gl) { > ip->i_iopen_gh.gh_gl->gl_object = NULL; which replaces a put with a queue & put if the queue fails (due to it being already on the queue) which doesn't look quite right to be since if calling gfs2_glock_put() was not safe before, then calling it conditionally like this is still no safer I think? Steve.