From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bw0-f46.google.com ([209.85.214.46]:56670 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757772Ab0KOPCC convert rfc822-to-8bit (ORCPT ); Mon, 15 Nov 2010 10:02:02 -0500 Received: by bwz15 with SMTP id 15so5255368bwz.19 for ; Mon, 15 Nov 2010 07:02:01 -0800 (PST) In-Reply-To: <1289744517-27949-1-git-send-email-bhalevy@panasas.com> References: <1289744517-27949-1-git-send-email-bhalevy@panasas.com> Date: Mon, 15 Nov 2010 10:02:00 -0500 Message-ID: Subject: Re: [PATCH] SQUASHME: pnfs-submit: encode layoutreturn on close before close From: Fred Isaman To: Benny Halevy Cc: linux-nfs@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Sun, Nov 14, 2010 at 9:21 AM, Benny Halevy wrote: > And handle errors from layoutcommit and layoutreturn on the reply path. > > Signed-off-by: Benny Halevy > --- >  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. 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@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html >