From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH, RFC] lockd: stop abusing file_lock_list Date: Thu, 16 Feb 2006 14:57:34 +0100 Message-ID: <20060216135734.GA3393@lst.de> References: <20060214192051.GA20751@lst.de> <1139956050.7867.64.camel@lade.trondhjem.org> <1140065071.8209.3.camel@lade.trondhjem.org> Mime-Version: 1.0 Content-Type: text/plain; charset=unknown-8bit Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Christoph Hellwig , linux-fsdevel@vger.kernel.org Return-path: Received: from verein.lst.de ([213.95.11.210]:57232 "EHLO mail.lst.de") by vger.kernel.org with ESMTP id S932266AbWBPN5h (ORCPT ); Thu, 16 Feb 2006 08:57:37 -0500 To: Trond Myklebust Content-Disposition: inline In-Reply-To: <1140065071.8209.3.camel@lade.trondhjem.org> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Wed, Feb 15, 2006 at 11:44:31PM -0500, Trond Myklebust wrote: > Author: Trond Myklebust > lockd: Fix Oopses due to list manipulation errors. >=20 > The patch "stop abusing file_lock_list introduces a couple of bugs si= nce > the locks may be copied and need to be removed from the lists when th= ey are > destroyed. >=20 > Signed-off-by: Trond Myklebust > --- >=20 > fs/lockd/clntproc.c | 9 ++++++++- > 1 files changed, 8 insertions(+), 1 deletions(-) >=20 > diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c > index 211113a..b1b924d 100644 > --- a/fs/lockd/clntproc.c > +++ b/fs/lockd/clntproc.c > @@ -447,12 +447,17 @@ static void nlmclnt_locks_copy_lock(stru > { > memcpy(&new->fl_u.nfs_fl, &fl->fl_u.nfs_fl, sizeof(new->fl_u.nfs_fl= )); > nlm_get_lockowner(new->fl_u.nfs_fl.owner); > + INIT_LIST_HEAD(&new->fl_u.nfs_fl.list); > + if (!list_empty(&fl->fl_u.nfs_fl.list)) > + list_add(&new->fl_u.nfs_fl.list, &fl->fl_u.nfs_fl.list); list_add initializes =E6nough so we don't need the INIT_LIST_HEAD in th= at case, so this could become: if (list_empty(&fl->fl_u.nfs_fl.list)) INIT_LIST_HEAD(&new->fl_u.nfs_fl.list); else list_add(&new->fl_u.nfs_fl.list, &fl->fl_u.nfs_fl.list); > * Remove from the granted list now so the lock doesn't get > * reclaimed while we're stuck in the unlock call. > */ > - list_del(&fl->fl_u.nfs_fl.list); > + if (!list_empty(&fl->fl_u.nfs_fl.list)) > + list_del_init(&fl->fl_u.nfs_fl.list); Probably should be just unconditionaly. list_del_init isn't a whole lo= t of instructions, but we save a branch and have more readable code. While we're at it, don't we need a INIT_LIST_HEAD in nlmclnt_locks_init_private aswell? - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html