[pnfs] [PATCH 3/3] pnfs: Don't send getattr with Data Server commit

William A. (Andy) Adamson androsadamson at gmail.com
Thu Jun 26 09:56:51 EDT 2008


On Thu, Jun 26, 2008 at 9:11 AM, Benny Halevy <bhalevy at panasas.com> wrote:

> On Jun. 26, 2008, 15:56 +0300, "William A. (Andy) Adamson" <
> androsadamson at gmail.com> wrote:
> >
> >
> > On Thu, Jun 26, 2008 at 4:58 AM, Benny Halevy <bhalevy at panasas.com
> > <mailto:bhalevy at panasas.com>> wrote:
> >
> >     On Jun. 23, 2008, 23:30 +0300, andros at netapp.com
> >     <mailto:andros at netapp.com> wrote:
> >     > From: Andy Adamson <andros at netapp.com <mailto:andros at netapp.com>>
> >     >
> >     > Add a flag to the file layout specific part of struct
> nfs_write_data
> >     > to indicate whether to send a commit  with at getattr to the MDS or
> >     > to send a commit without a getattr to a data server.
> >     >
> >     > Signed-off-by: Andy Adamson<andros at netapp.com
> >     <mailto:andros at netapp.com>>
> >     > ---
> >     >  fs/nfs/nfs4filelayout.c |    1 +
> >     >  fs/nfs/nfs4proc.c       |   24 ++++++++++++++++++-
> >     >  fs/nfs/nfs4xdr.c        |   57
> >     +++++++++++++++++++++++++++++++++++++++++++++++
> >     >  include/linux/nfs4.h    |    1 +
> >     >  include/linux/nfs_xdr.h |    2 +
> >     >  5 files changed, 83 insertions(+), 2 deletions(-)
> >     >
> >     > diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> >     > index c787e56..bdf0f8a 100644
> >     > --- a/fs/nfs/nfs4filelayout.c
> >     > +++ b/fs/nfs/nfs4filelayout.c
> >     > @@ -593,6 +593,7 @@ filelayout_commit(struct pnfs_layout_type
> >     *layoutid, int sync,
> >     >       dprintk("%s data %p pnfs_client %p nfslay %p sync %d\n",
> >     >               __func__, data, data->fldata.pnfs_client, nfslay,
> sync);
> >     >
> >     > +     data->fldata.commit_through_mds = nfslay->commit_through_mds;
> >     >       if (nfslay->commit_through_mds) {
> >     >               dprintk("%s data %p commit through mds\n", __func__,
> >     data);
> >     >               return 1;
> >     > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >     > index ed2c475..18492ee 100644
> >     > --- a/fs/nfs/nfs4proc.c
> >     > +++ b/fs/nfs/nfs4proc.c
> >     > @@ -3131,13 +3131,33 @@ static int nfs4_commit_done(struct
> >     rpc_task *task, struct nfs_write_data *data)
> >     >  static void nfs4_proc_commit_setup(struct nfs_write_data *data,
> >     struct rpc_message *msg)
> >     >  {
> >     >       struct nfs_server *server = NFS_SERVER(data->inode);
> >     > -
> >     > +
> >
> >     minor nit: I'd leave whitespace cleanups of existing code out.
> >     Since this is out of scope for this patch.
> >
> >     >       data->args.bitmask = server->attr_bitmask;
> >     >       data->res.server = server;
> >     >       msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_COMMIT];
> >     >  }
> >     >
> >     >  #if defined(CONFIG_PNFS)
> >     > +
> >     > +/*
> >     > + * pNFS doew not send a getattr to Data Serfers on commit.
> >
> >     another nit: "does", "Servers" (though I like data surfing :)
> >
> >
> > yikes!
> >
> >
> >
> >     > + */
> >     > +static void
> >     > +pnfs4_proc_commit_setup(struct nfs_write_data *data, struct
> >     rpc_message *msg)
> >     > +{
> >     > +     struct nfs_server *server = NFS_SERVER(data->inode);
> >     > +
> >     > +     dprintk("--> %s ds_nfs_client %p commit_through_mds %d\n",
> >     __func__,
> >     > +             data->fldata.ds_nfs_client,
> >     data->fldata.commit_through_mds);
> >     > +
> >     > +     if (!data->fldata.ds_nfs_client ||
> >     data->fldata.commit_through_mds)
> >     > +             return nfs4_proc_commit_setup(data, msg);
> >     > +
> >     > +     data->args.bitmask = server->attr_bitmask;
> >     > +     data->res.server = server;
> >     > +     msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_PNFS_COMMIT];
> >
> >     This needs to be converted to use nfs4_proc()
> >
> >
> > what do you mean?
>
> In be087ac9 nfs4_procedures changed so it can't be referenced directly
> and a function called nfs4_proc should be used to get to the rpc procedure
> instead (via  clp->cl_rpcclient->cl_procinfo that's set based on the rpc
> major and minor versions)


oh - i see. that patch wasn't in the tree when i wrote this patch...  :)


>
> I already took care of that when merging your patch...


thanks!

-->Andy

>
>
> Benny
>
> >
> >
> >
> >
> >     > +}
> >     > +
> >     >  /*
> >     >   * pNFS does not send a getattr on write.
> >     >   */
> >     > @@ -5577,7 +5597,7 @@ const struct nfs_rpc_ops pnfs_v41_clientops =
> {
> >     >       .read_done      = pnfs4_read_done,
> >     >       .write_setup    = pnfs4_proc_write_setup,
> >     >       .write_done     = pnfs4_write_done,
> >     > -     .commit_setup   = nfs4_proc_commit_setup,
> >     > +     .commit_setup   = pnfs4_proc_commit_setup,
> >     >       .commit_done    = pnfs4_commit_done,
> >     >       .file_open      = nfs_open,
> >     >       .file_release   = nfs_release,
> >     > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> >     > index 89581f4..2e202dc 100644
> >     > --- a/fs/nfs/nfs4xdr.c
> >     > +++ b/fs/nfs/nfs4xdr.c
> >     > @@ -780,6 +780,12 @@ static int nfs4_stat_to_errno(int);
> >     >                                       decode_sequence_maxsz + \
> >     >                                       decode_putfh_maxsz + \
> >     >                                       decode_write_maxsz)
> >     > +#define NFS41_enc_pnfs_commit_sz (compound_encode_hdr_maxsz + \
> >     > +                             encode_putfh_maxsz + \
> >     > +                             encode_commit_maxsz)
> >     > +#define NFS41_dec_pnfs_commit_sz (compound_decode_hdr_maxsz + \
> >     > +                             decode_putfh_maxsz + \
> >     > +                             decode_commit_maxsz)
> >     >  #endif /* CONFIG_PNFS */
> >     >
> >     >  static struct {
> >     > @@ -3442,6 +3448,31 @@ out:
> >     >  }
> >     >
> >     >  /*
> >     > + * Encode a PNFS COMMIT to Data Server request
> >     > + */
> >     > +static int nfs41_xdr_enc_pnfs_commit(struct rpc_rqst *req,
> >     uint32_t *p,
> >     > +                                 struct nfs_writeargs *args)
> >     > +{
> >     > +     struct xdr_stream xdr;
> >     > +     struct compound_hdr hdr = {
> >     > +             .nops = 3,
> >     > +     };
> >     > +     int status;
> >     > +
> >     > +     xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> >     > +     encode_compound_hdr(&xdr, &hdr, 1);
> >     > +     status = encode_sequence(&xdr, &args->seq_args);
> >     > +     if (status)
> >     > +             goto out;
> >     > +     status = encode_putfh(&xdr, args->fh);
> >     > +     if (status)
> >     > +             goto out;
> >     > +     status = encode_commit(&xdr, args);
> >     > +out:
> >     > +     return status;
> >     > +}
> >     > +
> >     > +/*
> >     >   *  Encode LAYOUTCOMMIT request
> >     >   */
> >     >  static int nfs41_xdr_enc_pnfs_layoutcommit(struct rpc_rqst *req,
> >     uint32_t *p,
> >     > @@ -7575,6 +7606,31 @@ out:
> >     >  }
> >     >
> >     >  /*
> >     > + * Decode PNFS COMMIT to Data Server response
> >     > + */
> >     > +static int nfs41_xdr_dec_pnfs_commit(struct rpc_rqst *rqstp,
> >     uint32_t *p,
> >     > +                                 struct nfs_writeres *res)
> >     > +{
> >     > +     struct xdr_stream xdr;
> >     > +     struct compound_hdr hdr;
> >     > +     int status;
> >     > +
> >     > +     xdr_init_decode(&xdr, &rqstp->rq_rcv_buf, p);
> >     > +     status = decode_compound_hdr(&xdr, &hdr);
> >     > +     if (status)
> >     > +             goto out;
> >     > +     status = decode_sequence(&xdr, &res->seq_res);
> >     > +     if (status)
> >     > +             goto out;
> >     > +     status = decode_putfh(&xdr);
> >     > +     if (status)
> >     > +             goto out;
> >     > +     status = decode_commit(&xdr, res);
> >     > +out:
> >     > +     return nfs4_fixup_status(status, hdr.status);
> >     > +}
> >     > +
> >     > +/*
> >     >   * Decode LAYOUTCOMMIT response
> >     >   */
> >     >  static int nfs41_xdr_dec_pnfs_layoutcommit(struct rpc_rqst
> >     *rqstp, uint32_t *p,
> >     > @@ -7823,6 +7879,7 @@ struct rpc_procinfo     nfs41_procedures[] =
> {
> >     >    PROC(PNFS_LAYOUTCOMMIT, enc_pnfs_layoutcommit,
> >      dec_pnfs_layoutcommit, 1),
> >     >    PROC(PNFS_LAYOUTRETURN, enc_pnfs_layoutreturn,
> >      dec_pnfs_layoutreturn, 1),
> >     >    PROC(PNFS_WRITE, enc_pnfs_write,  dec_pnfs_write, 1),
> >     > +  PROC(PNFS_COMMIT, enc_pnfs_commit,  dec_pnfs_commit, 1),
> >     >  #endif /* CONFIG_PNFS */
> >     >  };
> >     >  #endif /* CONFIG_NFS_V4_1 */
> >     > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> >     > index 33a2ce2..a5ab16d 100644
> >     > --- a/include/linux/nfs4.h
> >     > +++ b/include/linux/nfs4.h
> >     > @@ -485,6 +485,7 @@ enum {
> >     >       NFSPROC4_CLNT_PNFS_GETDEVICELIST,
> >     >       NFSPROC4_CLNT_PNFS_GETDEVICEINFO,
> >     >       NFSPROC4_CLNT_PNFS_WRITE,
> >     > +     NFSPROC4_CLNT_PNFS_COMMIT,
> >     >  #endif /* CONFIG_NFSD_V4_1 */
> >     >  };
> >     >
> >     > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> >     > index 8ace858..dccaf53 100644
> >     > --- a/include/linux/nfs_xdr.h
> >     > +++ b/include/linux/nfs_xdr.h
> >     > @@ -1049,6 +1049,7 @@ struct pnfs_fl_call_data {
> >     >       struct rpc_clnt         *pnfs_client;   /* Holds pNFS device
> >     across async calls */
> >     >       struct nfs_client       *ds_nfs_client;
> >     >       __u64                   orig_offset;
> >     > +     int                     commit_through_mds;
> >     >  };
> >     >  #endif /* CONFIG_PNFS */
> >     >
> >     > @@ -1077,6 +1078,7 @@ struct nfs_read_data {
> >     >  #if defined(CONFIG_PNFS)
> >     >  /* pnfsflag values */
> >     >  #define PNFS_NO_RPC          0x0001   /* non rpc result callback
> >     switch */
> >     > +#define PNFS_COMMIT_THROUGH_MDS      0x0002  /* commit through
> >     mds switch */
> >
> >     This isn't used in this patch.
> >
> >
> > sorry. left over from previous design
> >
> >
> >
> >     If ok with you, I'll take care of the comments...
> >
> >     Benny
> >
> >     >  #endif /* CONFIG_PNFS */
> >     >
> >     >  struct nfs_write_data {
> >
> >     _______________________________________________
> >     pNFS mailing list
> >     pNFS at linux-nfs.org <mailto:pNFS at linux-nfs.org>
> >     http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20080626/6839333a/attachment-0001.htm 


More information about the pNFS mailing list