[pnfs] device export ops

dean hildebrand seattleplus at gmail.com
Mon Oct 22 21:32:27 EDT 2007


On 10/22/07, Halevy, Benny <bhalevy at panasas.com> wrote:
> Aha, now I get the idea behind encdev.
> Let's just make it optional so that if the layout driver
> is not one of the standard ones it could use its own.

encdev is a function pointer, so it will be NULL if there isn't a
pre-existing encoding function.  Also, the underlying file system will
be responsable to call encdev, so it can ignore it if it so chooses.

Sounds good, hopefully I'll have time to get some patches together
this week that use this interface.
Dean

>
> Benny
>
>
> -----Original Message-----
> From: pnfs-bounces at linux-nfs.org on behalf of dean hildebrand
> Sent: Tue 2007-10-23 03:16
> To: pnfs at linux-nfs.org
> Subject: Re: [pnfs] device export ops
>
> On 10/22/07, Benny Halevy <bhalevy at panasas.com> wrote:
> > I wonder if you're not trying to cram too much into one function.
>
> Actually, you are right.  I was originally worried that the deviceid
> from getdeviceiter might not be valid in getdeviceinfo, but this is
> silly since we are iterating over cookie/verifier info, not deviceids.
>
> > What does encdev supposed to do?
>
> The encoding of the file layout should be done in nfsd since it is
> common to all file systems supporting file layout.  encdev is the
> encoding function for the file layout (very similar to how sys_readdir
> works)  Since the object and block layout formats will be standards as
> well, there is no reason why we can't use encdev to put the encoding
> of the block and object layouts into nfsd in the future.
>
> You missed this and the layout type in your data structures (since I
> haven't seen a spec that declares they are global, and I hope I never
> will), so how about:
>
> DATA STRUCTURES:
> typedef int (*nfsd41_encodedev_t)(struct nfsd4_export_xdr* xdr, void *device);
>
> struct nfsd4_export_xdr {
>      u32 *p;            /* in */
>      u32 *end;          /* in */
>      u32 bytes_written; /* out */
> };
>
> struct nfsd41_getdevinfo {
>      u32                             gdi_type;        /* request */
>      u32                             gdi_maxcount;    /* request */
>      u64                             gdi_devid;       /* request */
> };
>
> int (*get_device_info) (struct super_block *sb,
>                                      void *getdevinfo,  /* struct
> nfsd41_getdevinfo */
>                                      struct nfsd4_export_xdr *xdr,
>                                      nfsd41_encodedev_t encdev););
>
> struct nfsd41_getdevlistiter {
>      u32                             gdl_type;        /* request */
>      u64                             gdl_cookie;      /* req/resp */
>      u64                             gdl_verf;        /* req/resp */
>      u64                             gdl_devid;       /* resp */
> };
>
> int (*get_device_list_iter) (struct super_block *sb,
>                                          void *getdevlistiter,  /*
> struct nfsd4_pnfs_getdevlistiter */);
>
> Dean
>
> >
> > I think it makes better sense to have the API resemble the protocol
> > rather than a swiss knife function doing different things depending
> > on its inputs.
> > I'd rather have one method for iterating through the list and another for
> > encoding the device address body (da_addr_body), which is common to
> > GETDEVICEINFO and GETDEVICELIST.
> >
> >
> > struct nfsd4_export_xdr {
> >        u32 *p;            /* in */
> >        u32 *end;          /* in */
> >        u32 bytes_written; /* out */
> > };
> >
> > struct nfsd4_pnfs_getdevinfo {
> >        u32                             gdi_maxcount;    /* request */
> >        u64                             gdi_devid;       /* request */
> > };
> >
> > int (*get_device_info) (struct super_block *sb,
> >                        void *getdevinfo,  /* struct nfsd4_pnfs_getdevinfo */
> >                        struct nfsd4_export_xdr *xdr);
> >
> > struct nfsd4_pnfs_getdevlistiter {
> >        u64                             gdl_cookie;      /* req/resp */
> >        u64                             gdl_verf;        /* req/resp */
> >        u64                             gdl_devid;       /* resp */
> > };
> >
> > int (*get_device_listiter) (struct super_block *sb,
> >                            void *getdevlistiter,  /* struct nfsd4_pnfs_getdevinfo */);
> >
> > in the getdevicelist case I see the nfsd using get_device_listiter to iterate
> > through the devicelist, starting from the client given cookie and verf
> > and then continuing with the filesystem given cookie and verf.
> > The xdr function can prepare each entry's header (dli_id and da_layout_type)
> > and call get_device_info with the gdl_devid returned from get_device_listiter
> > and a calculated gdi_maxcount (based on the maxcount given by the client,
> > minus xdr space already consumed).
>
>
> > Benny
> >
> > On Oct. 18, 2007, 23:32 +0200, "dean hildebrand" <seattleplus at gmail.com> wrote:
> > > Hello,
> > >
> > > At Bakeathon we had a discussion regarding the device export ops.
> > > Beyond updating them for draft-13, we talked reducing the number of
> > > export ops and data copies it requires to retrieve device information.
> > >  The current ops retrieve the device, encode it, and them free it,
> > > which is quite cumbersome and requires extra data copies.  We noticed
> > > that readdir only has a single vfs op, so why can't we do something
> > > similar.  In addition, nfs readdir only calls this vfs op at xdr time,
> > > not in nfs4proc.
> > >
> > > To that end, Benny suggested moving to an iterator concept.  To
> > > retrieve a device list, NFSD continually calls the same export op,
> > > each time returning the next device in the list (using cookies, etc as
> > > an index)  Benny had suggested 2 ops, but I think we might need just
> > > one.
> > >
> > > To get feedback on the plan, here is the plan and the proposed data
> > > structures.  The main goals of this plan is to:
> > > a) Reduce the number of export ops
> > > b) Copy device information only once (onto the xdr buffer).
> > >
> > > The current 4 export ops related to devices would be replaced with a
> > > single *get_device* export op.  *get_device* would take as arguments:
> > > 1. superblock
> > > 2. nfsd4_pnfs_getdev struct: Filled with the client arguments.  For
> > > getdeviceinfo, it has a deviceid.  For getdevicelist, it has a valid
> > > cookie.
> > > 3. A xdr stream to encode the device
> > > 4. A function pointer, used mainly for the file layout type.  The
> > > function will be called by the underlying file system to encode a
> > > device on the xdr stream. (This is also similar to readdir)
> > >
> > > Essentially GETDEVICEINFO and GETDEVICELIST will do the same thing on
> > > the server except:
> > >
> > > For GETDEVICEINFO:
> > > 1. Set gd_devid to a non-zero value and call *get_device* once.  This
> > > will encode the device info for gd_devid onto the output stream. (This
> > > assumes devices cannot have a id of 0.  Is this ok?  If not, gd_devid
> > > could be a pointer.)
> >
> > I don't see such constraint in the protocol... so deviceid can be zero.
> >
> > >
> > > For GETDEVICELIST:
> > > 1. Set gd_device to 0 and set gd_cookie to be the value from the
> > > client. (If gd_device is a pointer, we would set it to NULL)
> > > 2. Call *get_device* multiple times to encode all the devices.  Stop
> > > when either nfserr_toosmall, nfserr_eof, or another error is returned.
> > >
> > >
> > > DATA STRUCTURES:
> > >
> > > typedef int (*nfsd41_encodedev_t)(struct nfsd4_export_xdr* xdr, void *device);
> > >
> > > struct nfsd4_export_xdr {
> > >         u32 *p;            /* in */
> > >         u32 *end;          /* in */
> > >         u32 bytes_written; /* out */
> > > };
> > >
> > > struct nfsd4_pnfs_devcookie {
> > >         u64                             gd_cookie;
> > >         u64                             gd_verf;
> > > };
> > >
> > > /* Filled in via the getdevicelist and getdeviceinfo arguments from
> > > the client */
> > > struct nfsd4_pnfs_getdev {
> > >         u32                             gd_type;           /* request */
> > >         u32                             gd_maxcount;    /* request */
> > >         u32                             gd_devid;          /* request */
> > >         struct nfsd4_pnfs_devcookie     gd_cookie;   /* req/resp */
> > > };
> > >
> > > /* pNFS:
> > >  * Args: sb - Current superblock
> > >  *       getdev - type, maxcount and either a devid or a cookie
> > >  *       xdr    - begin and end of xdr buffer, outputs num of bytes written
> > >  *       encdev - Function which does encoding of device
> > >  * Return: nfs_ok, nfserr_toosmall, nfserr, nfserr_eof */
> > > int (*get_device) (struct super_block *sb,
> > >                           void *getdev,  /* struct nfsd4_pnfs_getdev */
> > >                           struct nfsd4_export_xdr *xdr,
> > >                           nfsd41_encodedev_t encdev);
> > >
> > >
> > > Dean
> > > _______________________________________________
> > > pNFS mailing list
> > > pNFS at linux-nfs.org
> > > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >
> >
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>


More information about the pNFS mailing list