On Wed, Dec 17, 2008 at 01:14:53PM -0600, David Teigland wrote:
> 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@xxxxxxxxxxxx> 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.
Yep, that looks much better. Though actually I suspect what was really
intended was to use "flc" for the notifies, and "fl" for the
posix_lock_file().
Also, since flc is never actually handed to the posix lock system, I
think it should be a "shallow" lock copy--so it should be created with
__locks_copy_lock(). Something like the below?
--b.
diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index eba87ff..e8d9086 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -103,7 +103,7 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u64 number, struct file *file,
op->info.owner = (__u64) fl->fl_pid;
xop->callback = fl->fl_lmops->fl_grant;
locks_init_lock(&xop->flc);
- locks_copy_lock(&xop->flc, fl);
+ __locks_copy_lock(&xop->flc, fl);
xop->fl = fl;
xop->file = file;
} else {
@@ -173,8 +173,8 @@ static int dlm_plock_callback(struct plock_op *op)
}
/* got fs lock; bookkeep locally as well: */
- flc->fl_flags &= ~FL_SLEEP;
- if (posix_lock_file(file, flc, NULL)) {
+ fl->fl_flags &= ~FL_SLEEP;
+ if (posix_lock_file(file, fl, NULL)) {
/*
* This can only happen in the case of kmalloc() failure.
* The filesystem's own lock is the authoritative lock,
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
