From patchwork Thu Sep 8 10:00:25 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: tao.peng@emc.com X-Patchwork-Id: 1129302 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p88A0sMA005900 for ; Thu, 8 Sep 2011 10:00:57 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756812Ab1IHKAv (ORCPT ); Thu, 8 Sep 2011 06:00:51 -0400 Received: from mexforward.lss.emc.com ([128.222.32.20]:34413 "EHLO mexforward.lss.emc.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756461Ab1IHKAu convert rfc822-to-8bit (ORCPT ); Thu, 8 Sep 2011 06:00:50 -0400 Received: from hop04-l1d11-si03.isus.emc.com (HOP04-L1D11-SI03.isus.emc.com [10.254.111.23]) by mexforward.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id p88A0g6j030885 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 8 Sep 2011 06:00:42 -0400 Received: from mailhub.lss.emc.com (mailhubhoprd02.lss.emc.com [10.254.221.253]) by hop04-l1d11-si03.isus.emc.com (RSA Interceptor); Thu, 8 Sep 2011 06:00:27 -0400 Received: from mxhub24.corp.emc.com (mxhub24.corp.emc.com [128.221.56.110]) by mailhub.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id p88A0RRb011510; Thu, 8 Sep 2011 06:00:27 -0400 Received: from mx09a.corp.emc.com ([169.254.1.67]) by mxhub24.corp.emc.com ([128.221.56.110]) with mapi; Thu, 8 Sep 2011 06:00:26 -0400 From: To: , CC: , Date: Thu, 8 Sep 2011 06:00:25 -0400 Subject: RE: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release Thread-Topic: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release Thread-Index: Acxs4oYt0c7dIP6yRJSo+uX4vwo71QBKT37w Message-ID: References: <1314512558-16912-1-git-send-email-gusev.vitaliy@nexenta.com> <1315337382.16274.7.camel@lade.trondhjem.org> <4E669B21.30006@nexenta.com> In-Reply-To: <4E669B21.30006@nexenta.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 X-EMM-MHVC: 1 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Thu, 08 Sep 2011 10:00:58 +0000 (UTC) Hi, Gusev, > -----Original Message----- > From: Vitaliy Gusev [mailto:gusev.vitaliy@nexenta.com] > Sent: Wednesday, September 07, 2011 6:14 AM > To: Trond Myklebust > Cc: Vitaliy Gusev; Peng, Tao; linux-nfs@vger.kernel.org > Subject: Re: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release > > >> @@ -1376,7 +1376,8 @@ static void pnfs_list_write_lseg(struct inode *inode, > struct list_head *listp) > >> > >> list_for_each_entry(lseg,&NFS_I(inode)->layout->plh_segs, pls_list) { > >> if (lseg->pls_range.iomode == IOMODE_RW&& > >> - test_bit(NFS_LSEG_LAYOUTCOMMIT,&lseg->pls_flags)) > >> + test_bit(NFS_LSEG_LAYOUTCOMMIT,&lseg->pls_flags)&& > >> + list_empty(&lseg->pls_lc_list)) > >> list_add(&lseg->pls_lc_list, listp); > >> } > >> } > > > > If the lseg is already part of one layoutcommit, but we're sending a > > second one for the same range (presumably because we wrote more data in > > the same region), then the above causes the lseg to be excluded. > > > Yes, lseg is excluded, This patch does exactly only exclusion of lseg. > lseg is used here only to get/put reference on this lseg, so skipping is > correct. > > > However, checking on list_empty can occur (on other CPU) in the middle: > > list_del_init(&lseg->pls_lc_list); > Here >>>>>> > if (test_and_clear_bit(NFS_LSEG_LAYOUTCOMMIT, > &lseg->pls_flags)) > put_lseg(lseg); > > > So list_del_init must be executed under the same lock as > pnfs_list_write_lseg, i.e. inode->i_lock. Yes, you are right. How about following patch? From 14c6da67565fb31c2d2775ccefd93251f348910d Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Thu, 8 Sep 2011 00:57:02 -0400 Subject: [PATCH] nfsv4: fix race in layoutcommit lseg list create/free Since there can be more than one layoutcommit proc happen the same time, lseg list create/free should be protected. Otherwise lseg list may get corrupted. Reported-by: Vitaliy Gusev Signed-off-by: Peng Tao --- fs/nfs/nfs4proc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 8c77039..da7c20c 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -5964,6 +5964,7 @@ static void nfs4_layoutcommit_release(void *calldata) struct pnfs_layout_segment *lseg, *tmp; pnfs_cleanup_layoutcommit(data); + spin_lock(&data->args.inode->i_lock); /* Matched by references in pnfs_set_layoutcommit */ list_for_each_entry_safe(lseg, tmp, &data->lseg_list, pls_lc_list) { list_del_init(&lseg->pls_lc_list); @@ -5971,6 +5972,7 @@ static void nfs4_layoutcommit_release(void *calldata) &lseg->pls_flags)) put_lseg(lseg); } + spin_unlock(&data->args.inode->i_lock); put_rpccred(data->cred); kfree(data); }