[pnfs] [PATCH 4/12] use kref for nfsd nfs4_client

[Benny Halevy]

William A. (Andy) Adamson wrote:
> On 5/9/07, Benny Halevy <bhalevy at panasas.com> wrote:
>> J. Bruce Fields wrote:
>>> On Tue, May 08, 2007 at 10:26:12PM +0300, Benny Halevy wrote:
>>>
>>>> So the first step can be justified in mainline as in
>> nfsd4_probe_callback's
>>>> out_rpciod error case atomic_dec(&clp->cl_count) won't free up the
>> client
>>>> structure if it was expired concurrently.  expire_client shuts down the
>>>> callback rpc path so that may wait for nfsd4_probe_callback to finish
>>>> but I'm not sure.  At any rate, calling put_nfs4_client() there is
>>>> more robust.
>>>>
>>> Yes, the destruction of the rpc_client shouldn't succeed until any
>>> outstanding tasks are done.  But I agree that the reference counting
>>> there is at best odd.
>>>
>>> Actually, the caller (setclientid_confirm) also works with this client
>>> without a reference; if the client could be expired at any time, then
>>> that would already be a problem for it.  So I suppose it's just
>>> depending on the state lock to prevent the client expiration.  We're
>>> taking the reference in nfsd4_probe_callback() because we know it may be
>>> held after the state lock is dropped.
>>>
>> I think that the synchronization between expire_client and nfsd4_cb_recall
>> is broken if the callback is done not under the state_lock, even if the
>> caller of the callback has a reference on the client since the rpc_clnt in
>> clp->cl_callback.cb_client can go away before the callback rpc starts.
> 
> 
> good point. bump both the rpc_clnt->cl_count as well as the
> nfs4_client->cl_count in nfsd_break_deleg_cb() prior to the kthread_run and
> then call put_nfs4_client() and rpc_destroy_client() at the end of
> nfs4_cb_recall().
<snip>

I'm afraid that rpc_destroy_client() is not the right api to call.
Ideally, the callback process would take a ref on clnt->cl_users and call
rpc_release_client when the call is done.  This will synchronize correctly
with rpc_shutdown_client *but* rpc_release_client is not exported by
the sunrpc module.  Since the synchronization with outstanding callbacks
is supposed to happen anyway, shutting down the callback rpc client
only when the client's refcount goes to zero should work as well.

The attached patch is against mainline nfs-2.6, if you're good with it
I can send it also to linux-nfs for review.  It's passing connectathon tests
plus going through expire_client via the nfs4 laundromat, but I haven't
tested cb_recall.

Benny

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-nfs4_client-ref-count-for-cb_recall.patch
Url: http://linux-nfs.org/pipermail/pnfs/attachments/20070511/2d568165/attachment.diff