On Sat, 2 Aug 2014 10:47:27 -0400
Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Sat, Aug 2, 2014 at 10:20 AM, Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
> > On 8/2/2014 22:05, Trond Myklebust wrote:
> >> On Sat, Aug 2, 2014 at 9:43 AM, Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
> >>> On 8/2/2014 21:11, Trond Myklebust wrote:
> >>>> On Sat, Aug 2, 2014 at 6:39 AM, Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
> >>>>> On 7/30/2014 09:34, Jeff Layton wrote:
> >>>>>> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >>>>>>
> >>>>>> Preparation for removing the client_mutex.
> >>>>>>
> >>>>>> Convert the open owner hash table into a per-client table and protect it
> >>>>>> using the nfs4_client->cl_lock spin lock.
> >>>>>>
> >>>>>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >>>>>> ---
> >>>>>> fs/nfsd/netns.h | 1 -
> >>>>>> fs/nfsd/nfs4state.c | 187 ++++++++++++++++++++++++----------------------------
> >>>>>> fs/nfsd/state.h | 1 +
> >>>>>> 3 files changed, 86 insertions(+), 103 deletions(-)
> >>>>>>
> >>>>>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> >>>>>> index a71d14413d39..e1f479c162b5 100644
> >>>>>> --- a/fs/nfsd/netns.h
> >>>>>> +++ b/fs/nfsd/netns.h
> >>>>>> @@ -63,7 +63,6 @@ struct nfsd_net {
> >>>>>> struct rb_root conf_name_tree;
> >>>>>> struct list_head *unconf_id_hashtbl;
> >>>>>> struct rb_root unconf_name_tree;
> >>>>>> - struct list_head *ownerstr_hashtbl;
> >>>>>
> >>>>> I send a patch "NFSD: Rervert "knfsd: locks: flag NFSv4-owned locks"" before,
> >>>>> http://comments.gmane.org/gmane.linux.nfs/64382
> >>>>>
> >>>>> nfsd needs the hashtbl to find the lockowner for locking by owner from
> >>>>> fl->fl_owner stored in struct file_lock, but without nfs_client.
> >>>>
> >>>> Why? We're not currently doing that.
> >>>
> >>> Although not doing that right now, but there is a bug for getting the right ld_owner
> >>> in nfs4_set_lock_denied.
> >>>
> >>> If denying locks, vfs don't copy fl->fl_lmops to the returned file_lock, so that,
> >>> fl->fl_lmops always be NULL, nfsd never return the owner who holds the conflock.
> >>>
> >>> If we want fix this problem, needs finding the lockowner from struct file_lock.
> >>
> >> Do we really care enough about fixing nfs4_set_lock_denied enough to
> >> do so at the cost of reducing overall scalability of locking state?
> >
> > I just report this problem, don't think enough about the scalability.
> >
> >> We will always be faking up the clientid etc for local locks. Are
> >> there any clients out there that actually inspect the clientid on a
> >> result of NFS4ERR_DENIED and that will break if we give them a fake
> >> for non-local locks?
> >
> > Jeff has point the same problem of a non-nfs4_lockowner.
> > Maybe we should copy fl_lmops to conflock as before, nfsd can distinguish
> > the lockowner stored in struct file_lock by checking fl_lmops.
> >
>
> Alternatively, set a flag in fl_flags. Back in the days, we used to
> have a FL_NFSD, perhaps it is time to resurrect that?
>
Would we need a similar flag for lockd too?
I'm not sure a flag is the correct approach for this. A "fl_lmtype" field
or something similar might make sense, but I'm not sure that really
adds much over just ensuring that fl_lmops is set properly for these locks.
That said, we definitely will need to ensure that there are no harmful
effects from setting fl_lmops on a conflock container. I don't see any
right offhand, but it's probably reasonable to put something like that
in -next for a bit of soak time...
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
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
