Re: [PATCH] lockd: handle fl_grant callbacks with coalesced locks (RFC)

[David Teigland <teigland@redhat.com>]

On Tue, Dec 16, 2008 at 04:11:58PM -0500, Jeff Layton wrote:
> On Tue, 16 Dec 2008 14:56:35 -0500
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > After thinking a little more: the interface is a lot simpler if it's
> > just a simple request and reply (with the reply for a lock identical to
> > the request).  I believe that's more or less what gfs2 is already do
> > internally, if we look at the posix lock upcalls it's making to
> > userspace.  So it shouldn't be hard to fix gfs2 to hand us back a lock
> > that doesn't take into account any coalescing.  If it needs to keep an
> > extra (unmodified) copy of the lock around, that's OK.
> > 
> > Did you try that and find a reason that doesn't work?
> > 
> > --b.
> > 
> 
> Agreed. That would be much simpler, I think...
> 
> I didn't try that, though I did consider it before wandering down the
> rabbit hole. Dave, any thoughts?

Jeff suggested the following patch, which I've tried and it fixes the
problem I was seeing.  It passes the original, unmodified file_lock to
notify(), instead of the copy which is passed to (and coalesced by)
posix_lock_file().  I'm guessing this was reason for having a copy of the
file_lock in the first place, but it was just not used correctly.

diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index eba87ff..502b1ea 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -168,7 +168,7 @@ static int dlm_plock_callback(struct plock_op *op)
 	notify = xop->callback;
 
 	if (op->info.rv) {
-		notify(flc, NULL, op->info.rv);
+		notify(fl, NULL, op->info.rv);
 		goto out;
 	}
 
@@ -187,7 +187,7 @@ static int dlm_plock_callback(struct plock_op *op)
 			  (unsigned long long)op->info.number, file, fl);
 	}
 
-	rv = notify(flc, NULL, 0);
+	rv = notify(fl, NULL, 0);
 	if (rv) {
 		/* XXX: We need to cancel the fs lock here: */
 		log_print("dlm_plock_callback: lock granted after lock request "