On Jan 18, 2013, at 4:59 PM, Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote:
> On 01/18/2013 01:33 PM, Chuck Lever wrote:
>>
>> On Jan 18, 2013, at 4:28 PM, Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote:
>>
>>> Any chance the STALE_CLIENTID case needs a 'break'?
>>
>> I don't think so. LEASE_CONFIRM is set, and we want to wake the state renewal thread.
>>
>>>
>>> Twice I've seen kernel crashes after the nfs40_walk_client_list
>>> failed (though code comments say it should never fail).
>>
>> nfs40_walk_client_list() is looking for an nfs_client that is supposed to already be in the nfs_client list. If the search fails, that's a bug.
>>
>> Eyeball the contents of your nfs_client list. You should find an appropriate nfs_client in there, and then figure out why the search doesn't find it.
>
> Ok, I think I found another issue.
>
> nfs4_client_init does not initialize 'old', but passes it to nfs4_discover_server_trunking.
>
> That gets passed to the detect_trunking operation, which is nfs40_discover_server_trunking
> or nfs41_discover_server_trunking.
Yes, *result is an output parameter, not an input parameter.
> This will call walk_client_list, which also may not ever assign a value to
> 'result'.
It should always assign *result in the SUCCESS case.
> The code in walk_client_list always dereferences result, however.
>
> So, that is probably why my system blows up shortly after the 'impossible'
> error message...
In particular, nfs4_walk_client_list() does not set *result when it returns NFS4ERR_STALE_CLIENTID. In nfs4_discover_server_trunking(), should we therefore have this:
status = nfs40_walk_client_list(clp, result, cred);
switch (status) {
case -NFS4ERR_STALE_CLIENTID:
set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
>> nfs4_schedule_state_renewal(clp);
>> break;
case 0:
I'm not sure the nfs4_schedule_state_renewal() call is necessary here.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
