On Mon, 2014-08-11 at 12:19 -0400, Jeff Layton wrote:
> On Sun, 10 Aug 2014 23:38:25 +0800
> Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
>
> > Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using
> > fl_lmops field in file_lock for checking nfsd4 lockowner.
> >
> > But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return
> > of conflicting locks) causes the fl_lmops of conflock always be NULL.
> >
> > Also, commit 0996905f93 (lockd: posix_test_lock() should not call
> > locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.
> >
> > v2: Only change the order from 3/3 to 1/3 now.
> >
> > Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
> > ---
> > fs/lockd/svclock.c | 2 +-
> > fs/locks.c | 25 ++++++-------------------
> > include/linux/fs.h | 6 ------
> > 3 files changed, 7 insertions(+), 26 deletions(-)
> >
> > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > index ab798a8..e1f209c 100644
> > --- a/fs/lockd/svclock.c
> > +++ b/fs/lockd/svclock.c
> > @@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
> > block->b_flags |= B_TIMED_OUT;
> > if (conf) {
> > if (block->b_fl)
> > - __locks_copy_lock(block->b_fl, conf);
> > + locks_copy_lock(block->b_fl, conf);
> > }
> > }
> >
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 717fbc4..91b0f03 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
> > new->fl_lmops = fl->fl_lmops;
> > }
> >
> > -/*
> > - * Initialize a new lock from an existing file_lock structure.
> > - */
> > -void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
> > +void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
> > {
> > + locks_release_private(new);
> > +
> > new->fl_owner = fl->fl_owner;
> > new->fl_pid = fl->fl_pid;
> > - new->fl_file = NULL;
> > + new->fl_file = fl->fl_file;
> > new->fl_flags = fl->fl_flags;
> > new->fl_type = fl->fl_type;
> > new->fl_start = fl->fl_start;
> > new->fl_end = fl->fl_end;
> > new->fl_ops = NULL;
> > new->fl_lmops = NULL;
> > -}
> > -EXPORT_SYMBOL(__locks_copy_lock);
> > -
> > -void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
> > -{
> > - locks_release_private(new);
> > -
> > - __locks_copy_lock(new, fl);
> > - new->fl_file = fl->fl_file;
> > - new->fl_ops = fl->fl_ops;
> > - new->fl_lmops = fl->fl_lmops;
> >
> > locks_copy_private(new, fl);
> > }
>
> (cc'ing Joe Perches)
(cc'ing Andrew Morton too)
> Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there
> is that you now need to ensure that any conflock structures are
> properly initialized before passing them to locks_copy_lock.
>
> The nfsv4 server code currently doesn't do that and it will need to be
> fixed to do so or that will be a regression.
>
> For the NLM code, Joe Perches has proposed a patch to remove the
> conflock parameter from lm_grant since the callers always pass in NULL
> anyway. You may want to pull in his patch and rebase yours on top of it
> since it'll remove that __locks_copy_lock call altogether.
>
> Joe, is Andrew merging that patch or do I need to pull it into the
> locks tree?
I believe Andrew is merging it.
--
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
