Re: [PATCH, RFC] lockd: stop abusing file_lock_list

[Trond Myklebust <trond.myklebust@fys.uio.no>]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

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


[-- Attachment #2: linux-2.6.16-62-fixup_lockd_lists.dif --]
[-- Type: text/plain, Size: 1822 bytes --]

Author: Trond Myklebust <Trond.Myklebust@netapp.com>
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 <Trond.Myklebust@netapp.com>
---

 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,