On Thu, Sep 28, 2017 at 3:50 PM, Anna Schumaker
<schumaker.anna@xxxxxxxxx> wrote:
> Hi Olga,
>
> On 09/28/2017 01:28 PM, Olga Kornievskaia wrote:
>> It's possible that server replies back with CB_OFFLOAD call and
>> COPY reply at the same time such that client will process
>> CB_OFFLOAD before reply to COPY. For that keep a list of pending
>> callback stateids received and them before waiting on completion
> ^^^^^^^^^^^^^^^^^^^^^^^^
> What are you doing with the stateids?
Is fixing "them" to "then" address the question?
>> check the pending list.
>>
>> Cleanup any pending copies on the client shutdown.
>>
>> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
>> ---
>> fs/nfs/callback_proc.c | 17 ++++++++++++++---
>> fs/nfs/nfs42proc.c | 24 +++++++++++++++++++++---
>> fs/nfs/nfs4client.c | 15 +++++++++++++++
>> include/linux/nfs_fs_sb.h | 1 +
>> 4 files changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index d3e7b61..d546c9f 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -676,11 +676,12 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>> struct cb_offloadargs *args = data;
>> struct nfs_server *server;
>> struct nfs4_copy_state *copy;
>> + bool found = false;
>>
>> + spin_lock(&cps->clp->cl_lock);
>> rcu_read_lock();
>> list_for_each_entry_rcu(server, &cps->clp->cl_superblocks,
>> client_link) {
>> - spin_lock(&server->nfs_client->cl_lock);
>> list_for_each_entry(copy, &server->ss_copies, copies) {
>> if (memcmp(args->coa_stateid.other,
>> copy->stateid.other,
>> @@ -688,13 +689,23 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>> continue;
>> nfs4_copy_cb_args(copy, args);
>> complete(©->completion);
>> - spin_unlock(&server->nfs_client->cl_lock);
>> + found = true;
>> goto out;
>> }
>> - spin_unlock(&server->nfs_client->cl_lock);
>> }
>> out:
>> rcu_read_unlock();
>> + if (!found) {
>> + copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>> + if (!copy) {
>> + spin_unlock(&cps->clp->cl_lock);
>> + return -ENOMEM;
>> + }
>> + memcpy(©->stateid, &args->coa_stateid, NFS4_STATEID_SIZE);
>> + nfs4_copy_cb_args(copy, args);
>> + list_add_tail(©->copies, &cps->clp->pending_cb_stateids);
>> + }
>> + spin_unlock(&cps->clp->cl_lock);
>>
>> return 0;
>> }
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index c8ca353..d0f454e 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -137,15 +137,32 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>> uint64_t *ret_count)
>> {
>> struct nfs4_copy_state *copy;
>> - int status;
>> + int status = NFS4_OK;
>
> Might as well just move this bit to the previous patch :)
Got it.
> Anna
>
>> + bool found_pending = false;
>> +
>> + spin_lock(&server->nfs_client->cl_lock);
>> + list_for_each_entry(copy, &server->nfs_client->pending_cb_stateids,
>> + copies) {
>> + if (memcmp(&res->write_res.stateid, ©->stateid,
>> + NFS4_STATEID_SIZE))
>> + continue;
>> + found_pending = true;
>> + list_del(©->copies);
>> + break;
>> + }
>> + if (found_pending) {
>> + spin_unlock(&server->nfs_client->cl_lock);
>> + goto out;
>> + }
>>
>> copy = kzalloc(sizeof(struct nfs4_copy_state), GFP_NOFS);
>> - if (!copy)
>> + if (!copy) {
>> + spin_unlock(&server->nfs_client->cl_lock);
>> return -ENOMEM;
>> + }
>> memcpy(©->stateid, &res->write_res.stateid, NFS4_STATEID_SIZE);
>> init_completion(©->completion);
>>
>> - spin_lock(&server->nfs_client->cl_lock);
>> list_add_tail(©->copies, &server->ss_copies);
>> spin_unlock(&server->nfs_client->cl_lock);
>>
>> @@ -153,6 +170,7 @@ static int handle_async_copy(struct nfs42_copy_res *res,
>> spin_lock(&server->nfs_client->cl_lock);
>> list_del_init(©->copies);
>> spin_unlock(&server->nfs_client->cl_lock);
>> +out:
>> *ret_count = copy->count;
>> memcpy(&res->write_res.verifier, ©->verf, sizeof(copy->verf));
>> if (copy->count <= 0)
>> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
>> index e9bea90..46d4649 100644
>> --- a/fs/nfs/nfs4client.c
>> +++ b/fs/nfs/nfs4client.c
>> @@ -156,9 +156,23 @@ struct rpc_clnt *
>> }
>> }
>>
>> +static void
>> +nfs4_cleanup_callback(struct nfs_client *clp)
>> +{
>> + struct nfs4_copy_state *cp_state;
>> +
>> + while (!list_empty(&clp->pending_cb_stateids)) {
>> + cp_state = list_entry(clp->pending_cb_stateids.next,
>> + struct nfs4_copy_state, copies);
>> + list_del(&cp_state->copies);
>> + kfree(cp_state);
>> + }
>> +}
>> +
>> void nfs41_shutdown_client(struct nfs_client *clp)
>> {
>> if (nfs4_has_session(clp)) {
>> + nfs4_cleanup_callback(clp);
>> nfs4_shutdown_ds_clients(clp);
>> nfs4_destroy_session(clp->cl_session);
>> nfs4_destroy_clientid(clp);
>> @@ -202,6 +216,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
>> #if IS_ENABLED(CONFIG_NFS_V4_1)
>> init_waitqueue_head(&clp->cl_lock_waitq);
>> #endif
>> + INIT_LIST_HEAD(&clp->pending_cb_stateids);
>> return clp;
>>
>> error:
>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>> index 511eefb..72f159e 100644
>> --- a/include/linux/nfs_fs_sb.h
>> +++ b/include/linux/nfs_fs_sb.h
>> @@ -119,6 +119,7 @@ struct nfs_client {
>> #endif
>>
>> struct net *cl_net;
>> + struct list_head pending_cb_stateids;
>> };
>>
>> /*
>>
> --
> 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
--
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
