On 2010-11-15 17:02, Fred Isaman wrote:
> On Sun, Nov 14, 2010 at 9:21 AM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote:
>> And handle errors from layoutcommit and layoutreturn on the reply path.
>>
>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>> ---
>> fs/nfs/nfs4xdr.c | 35 ++++++++++++++++++-----------------
>> fs/nfs/pnfs.c | 1 +
>> 2 files changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index 1804f35..0e6e5e4 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -441,17 +441,17 @@ static int nfs4_stat_to_errno(int);
>> #define NFS4_enc_close_sz (compound_encode_hdr_maxsz + \
>> encode_sequence_maxsz + \
>> encode_putfh_maxsz + \
>> - encode_close_maxsz + \
>> - encode_getattr_maxsz + \
>> + encode_layoutcommit_maxsz + \
>> encode_layoutreturn_maxsz + \
>> - encode_layoutcommit_maxsz)
>> + encode_close_maxsz + \
>> + encode_getattr_maxsz)
>> #define NFS4_dec_close_sz (compound_decode_hdr_maxsz + \
>> decode_sequence_maxsz + \
>> decode_putfh_maxsz + \
>> - decode_close_maxsz + \
>> - decode_getattr_maxsz + \
>> + decode_layoutcommit_maxsz + \
>> decode_layoutreturn_maxsz + \
>> - decode_layoutcommit_maxsz)
>> + decode_close_maxsz + \
>> + decode_getattr_maxsz)
>> #define NFS4_enc_setattr_sz (compound_encode_hdr_maxsz + \
>> encode_sequence_maxsz + \
>> encode_putfh_maxsz + \
>> @@ -2160,10 +2160,10 @@ static int nfs4_xdr_enc_close(struct rpc_rqst *req, __be32 *p, struct nfs_closea
>> encode_putfh(&xdr, args->fh, &hdr);
>> if (args->op_bitmask & NFS4_HAS_LAYOUTCOMMIT) /* layoutcommit set */
>> encode_layoutcommit(&xdr, &args->lc_args, &hdr);
>> - encode_close(&xdr, args, &hdr);
>> - encode_getfattr(&xdr, args->bitmask, &hdr);
>> if (args->op_bitmask & NFS4_HAS_LAYOUTRETURN) /* layoutreturn set */
>> encode_layoutreturn(&xdr, &args->lr_args, &hdr);
>> + encode_close(&xdr, args, &hdr);
>> + encode_getfattr(&xdr, args->bitmask, &hdr);
>> encode_nops(&hdr);
>> return 0;
>> }
>> @@ -5743,9 +5743,16 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
>> status = decode_putfh(&xdr);
>> if (status)
>> goto out;
>> - /* We pay no attention to the layoutcommit return */
>> - if (res->op_bitmask & NFS4_HAS_LAYOUTCOMMIT)
>> - decode_layoutcommit(&xdr);
>> + if (res->op_bitmask & NFS4_HAS_LAYOUTCOMMIT) {
>> + status = decode_layoutcommit(&xdr);
>> + if (status)
>> + goto out;
>> + }
>> + if (res->op_bitmask & NFS4_HAS_LAYOUTRETURN) {
>> + status = decode_layoutreturn(&xdr, &res->lr_res);
>> + if (status)
>> + goto out;
>
> What prevents infinite loop here? With LAYOUTCOMMIT, the inode data
> is cleared so that on retry it will not be called. I see no
> comparable "pre-cleaning" done for LAYOUTRETURN.
>
pnfs_roc will find no layout segment to return the second time around.
Benny
> Fred
>
>> + }
>> status = decode_close(&xdr, res);
>> if (status != 0)
>> goto out;
>> @@ -5757,12 +5764,6 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, __be32 *p, struct nfs_clos
>> */
>> decode_getfattr(&xdr, res->fattr, res->server,
>> !RPC_IS_ASYNC(rqstp->rq_task));
>> - /*
>> - * With the forgetful model, we pay no attention to the
>> - * layoutreturn status.
>> - */
>> - if (res->op_bitmask & NFS4_HAS_LAYOUTRETURN)
>> - decode_layoutreturn(&xdr, &res->lr_res);
>> out:
>> return status;
>> }
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index 15673d0..90a868b 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -640,6 +640,7 @@ pnfs_roc(struct nfs4_closedata *data)
>> LIST_HEAD(tmp_list);
>> bool found = false;
>>
>> + data->arg.op_bitmask = data->res.op_bitmask = 0;
>> spin_lock(&data->inode->i_lock);
>> lo = NFS_I(data->inode)->layout;
>> if (!lo || lo->roc_iomode == 0 ||
>> --
>> 1.7.2.3
>>
>> --
>> 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
