[pnfs] device export ops

[Benny Halevy]

I wonder if you're not trying to cram too much into one function.
What does encdev supposed to do?
Does the caller of get_device supposed to iterate on several
devices?

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