From patchwork Fri May 13 17:42:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 9092751 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6CF319F1C1 for ; Fri, 13 May 2016 17:43:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 81B4C20218 for ; Fri, 13 May 2016 17:43:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 573BE2017D for ; Fri, 13 May 2016 17:42:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753394AbcEMRm6 (ORCPT ); Fri, 13 May 2016 13:42:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54808 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751573AbcEMRm5 (ORCPT ); Fri, 13 May 2016 13:42:57 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4168285364; Fri, 13 May 2016 17:42:57 +0000 (UTC) Received: from nux.redhat.com (vpn1-6-107.ams2.redhat.com [10.36.6.107]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4DHgUI4006938; Fri, 13 May 2016 13:42:54 -0400 From: Andreas Gruenbacher To: Alexander Viro , Bob Peterson , Steven Whitehouse Cc: Andreas Gruenbacher , cluster-devel@redhat.com, linux-fsdevel@vger.kernel.org Subject: [PATCH 7/7] GFS2: Prevent deadlock in gfs2_lookup_by_inum Date: Fri, 13 May 2016 19:42:29 +0200 Message-Id: <1463161349-547-8-git-send-email-agruenba@redhat.com> In-Reply-To: <1463161349-547-1-git-send-email-agruenba@redhat.com> References: <1463161349-547-1-git-send-email-agruenba@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 13 May 2016 17:42:57 +0000 (UTC) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In gfs2_lookup_by_inum, we must take the glock of a presumed inode before we can determine if the block indeed still contains an inode, then look up the inode. When a lookup finds an inode that is being freed, it usually waits until that inodes has gone before returning. However, freeing the inode requires taking the inode glock, so we end up deadlocking. Fix that by changing gfs2_inode_lookup: instead of waiting for inodes that are being freed, return the context necessary for waiting. Then, in gfs2_lookup_by_inum, drop the glock before waiting and retrying the lookup. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/dir.c | 2 +- fs/gfs2/inode.c | 42 +++++++++++++++++++++++++----------------- fs/gfs2/inode.h | 2 +- fs/gfs2/ops_fstype.c | 2 +- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c index d4014af..eb54888 100644 --- a/fs/gfs2/dir.c +++ b/fs/gfs2/dir.c @@ -1660,7 +1660,7 @@ struct inode *gfs2_dir_search(struct inode *dir, const struct qstr *name, brelse(bh); if (fail_on_exist) return ERR_PTR(-EEXIST); - inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino, 0); + inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino, NULL); if (!IS_ERR(inode)) GFS2_I(inode)->i_rahead = rahead; return inode; diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 1fa4799..fb7ccb0 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -85,30 +85,29 @@ struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr) } static struct inode *gfs2_iget(struct super_block *sb, u64 no_addr, - int non_block) + struct gfs2_freeing_inode *freeing) { - struct gfs2_freeing_inode freeing; - struct gfs2_match match = { - .no_addr = no_addr, - .freeing = &freeing, - }; + struct gfs2_freeing_inode wait_here; + struct gfs2_match match; struct inode *inode; + if (!freeing) + freeing = &wait_here; + match.no_addr = no_addr; + match.freeing = freeing; while (1) { - freeing.wq = NULL; + freeing->wq = NULL; inode = find_inode_nowait(sb, no_addr, gfs2_match_inode, &match); if (inode) { wait_on_inode(inode); return inode; } - if (freeing.wq) { - if (non_block) { - finish_wait(freeing.wq, &freeing.bit_wait.wait); + if (freeing->wq) { + if (freeing != &wait_here) return ERR_PTR(-EAGAIN); - } schedule(); - finish_wait(freeing.wq, &freeing.bit_wait.wait); + finish_wait(freeing->wq, &freeing->bit_wait.wait); continue; } @@ -162,22 +161,23 @@ static void gfs2_set_iop(struct inode *inode) /** * gfs2_inode_lookup - Lookup an inode * @sb: The super block - * @no_addr: The inode number * @type: The type of the inode - * non_block: Can we block on inodes that are being freed? + * @no_addr: The inode number + * @freeing: Filled in when inode is being freed * * Returns: A VFS inode, or an error */ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, - u64 no_addr, u64 no_formal_ino, int non_block) + u64 no_addr, u64 no_formal_ino, + struct gfs2_freeing_inode *freeing) { struct inode *inode; struct gfs2_inode *ip; struct gfs2_glock *io_gl = NULL; int error; - inode = gfs2_iget(sb, no_addr, non_block); + inode = gfs2_iget(sb, no_addr, freeing); if (IS_ERR(inode)) return inode; ip = GFS2_I(inode); @@ -237,11 +237,13 @@ fail: struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, u64 *no_formal_ino, unsigned int blktype) { + struct gfs2_freeing_inode freeing; struct super_block *sb = sdp->sd_vfs; struct gfs2_holder i_gh; struct inode *inode = NULL; int error; +repeat: /* Must not read in block until block type is verified */ error = gfs2_glock_nq_num(sdp, no_addr, &gfs2_inode_glops, LM_ST_EXCLUSIVE, GL_SKIP, &i_gh); @@ -252,7 +254,13 @@ struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, if (error) goto fail; - inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0, 1); + inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0, &freeing); + if (inode == ERR_PTR(-EAGAIN)) { + gfs2_glock_dq_uninit(&i_gh); + schedule(); + finish_wait(freeing.wq, &freeing.bit_wait.wait); + goto repeat; + } if (IS_ERR(inode)) goto fail; diff --git a/fs/gfs2/inode.h b/fs/gfs2/inode.h index 4863513..f043601 100644 --- a/fs/gfs2/inode.h +++ b/fs/gfs2/inode.h @@ -100,7 +100,7 @@ struct gfs2_freeing_inode { extern struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned type, u64 no_addr, u64 no_formal_ino, - int non_block); + struct gfs2_freeing_inode *freeing); extern struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, u64 *no_formal_ino, unsigned int blktype); diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index dbed9e2..cfc4158 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -454,7 +454,7 @@ static int gfs2_lookup_root(struct super_block *sb, struct dentry **dptr, struct dentry *dentry; struct inode *inode; - inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0, 0); + inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0, NULL); if (IS_ERR(inode)) { fs_err(sdp, "can't read in %s inode: %ld\n", name, PTR_ERR(inode)); return PTR_ERR(inode);