On 8/2/2014 21:23, Jeff Layton wrote:
> On Sat, 2 Aug 2014 09:11:25 -0400
> Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> 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.
>>
>>> If moving the hashtbl to nfs_client, it's hard to finding the lockowner for locking.
>>
>
> I think there's a fundamental flaw in your original patch.
>
> You're casting fl->fl_owner to a struct nfs4_lockowner pointer, and
> then dereferencing that pointer to get to the lo->lo_hashval field. The
> problem there is that conflock might refer to a lock that is not a
> nfsv4 lock, and in that case there is zero guarantee that the
> fl_owner_t is a pointer at all, so that method could end up causing
> oopses.
Yes, you are right, the old patch has the problem.
>
> The fl_owner is really intended to be an opaque token, and you can't
> turn it back into a pointer without knowing for a fact what sort of
> lock it is. In the v4 case, we use the fl_lmops field to try and
> designate that.
Got it.
>
> Perhaps you need to change it so that the conflocks get fl_lmops set
> properly instead of changing things like you are in that patch?
OK, I will sends a new patch coping fl_lmops to conflocks for this problem.
thanks,
Kinglong Mee
>
--
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
