From mboxrd@z Thu Jan 1 00:00:00 1970 From: Trond Myklebust Subject: Re: [PATCH, RFC] lockd: stop abusing file_lock_list Date: Wed, 15 Feb 2006 23:44:31 -0500 Message-ID: <1140065071.8209.3.camel@lade.trondhjem.org> References: <20060214192051.GA20751@lst.de> <1139956050.7867.64.camel@lade.trondhjem.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-rrzk75PQaiv9No4sIS5I" Cc: linux-fsdevel@vger.kernel.org Return-path: Received: from pat.uio.no ([129.240.130.16]:47086 "EHLO pat.uio.no") by vger.kernel.org with ESMTP id S932231AbWBPEov (ORCPT ); Wed, 15 Feb 2006 23:44:51 -0500 To: Christoph Hellwig In-Reply-To: <1139956050.7867.64.camel@lade.trondhjem.org> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org --=-rrzk75PQaiv9No4sIS5I Content-Type: text/plain Content-Transfer-Encoding: 7bit On Tue, 2006-02-14 at 17:27 -0500, Trond Myklebust wrote: > On Tue, 2006-02-14 at 20:20 +0100, Christoph Hellwig wrote: > > Currently lockd directly access the file_lock_list from fs/locks.c. > > It does so to mark locks granted or reclaimable. This is very > > suboptimal, because a) lockd needs to poke into locks.c internals, and > > b) it needs to iterate over all locks in the system for marking locks > > granted or reclaimable. > > > > This patch adds lists for granted and reclaimable locks to the nlm_host > > structure instead, and adds locks to those. > > Thanks! From a quick eyeballing of the code, it looks good. > > I'll put it into the NFS_ALL stream for testing (and farm it out to > Andrew if all goes well). OK... After testing, it appears it will Oops without the attached patch. Cheers, Trond --=-rrzk75PQaiv9No4sIS5I Content-Disposition: inline; filename=linux-2.6.16-62-fixup_lockd_lists.dif Content-Type: text/plain; name=linux-2.6.16-62-fixup_lockd_lists.dif; charset=utf-8 Content-Transfer-Encoding: 7bit Author: Trond Myklebust lockd: Fix Oopses due to list manipulation errors. The patch "stop abusing file_lock_list introduces a couple of bugs since the locks may be copied and need to be removed from the lists when they are destroyed. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) 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); } static void nlmclnt_locks_release_private(struct file_lock *fl) { nlm_put_lockowner(fl->fl_u.nfs_fl.owner); fl->fl_ops = NULL; + if (!list_empty(&fl->fl_u.nfs_fl.list)) + list_del(&fl->fl_u.nfs_fl.list); } static struct file_lock_operations nlmclnt_lock_ops = { @@ -465,6 +470,7 @@ static void nlmclnt_locks_init_private(s BUG_ON(fl->fl_ops != NULL); fl->fl_u.nfs_fl.state = 0; fl->fl_u.nfs_fl.owner = nlm_find_lockowner(host, fl->fl_owner); + INIT_LIST_HEAD(&fl->fl_u.nfs_fl.list); fl->fl_ops = &nlmclnt_lock_ops; } @@ -621,7 +627,8 @@ nlmclnt_unlock(struct nlm_rqst *req, str * 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); if (req->a_flags & RPC_TASK_ASYNC) { status = nlmclnt_async_call(req, NLMPROC_UNLOCK, --=-rrzk75PQaiv9No4sIS5I--