Re: [pnfs] [PATCH 02/14] SQUASHME pnfs-submit: call get_device_info from filelayout_check

Benny Halevy
2010-04-12T13:14:36Z

On Apr. 08, 2010, 22:53 +0300, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
>
> filelayout_check() validates a LAYOUTGET return including the deviceid.
> If the deviceid is not cached, GETDEVICEINFO is called.
>
> Once cached, the deviceid will only be uncached upon umount or a return of all
> refering layouts. So, there is no need for a get_device_info call in the I/O
> paths.
>
> Remove nfs4_file_layout_dsaddr_get.
>
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
> fs/nfs/nfs4filelayout.c | 11 +++++++----
> fs/nfs/nfs4filelayout.h | 6 ++++--
> fs/nfs/nfs4filelayoutdev.c | 27 +++------------------------
> 3 files changed, 14 insertions(+), 30 deletions(-)
>
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index df6c1d0..ffc3ef2 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -413,12 +413,15 @@ filelayout_check_layout(struct pnfs_layout_type *lo,
> struct nfs_server *nfss = NFS_SERVER(PNFS_INODE(lo));
>
> dprintk("--> %s\n", __func__);
> - dsaddr = nfs4_file_layout_dsaddr_get(FILE_MT(PNFS_INODE(lo)),
> + dsaddr = nfs4_pnfs_device_item_find(FILE_MT(PNFS_INODE(lo))->hlist,
> &fl->dev_id);
> if (dsaddr == NULL) {
> - dprintk("%s NO device for dev_id %s\n",
> + dsaddr = get_device_info(FILE_MT(PNFS_INODE(lo)), &fl->dev_id);
> + if (dsaddr == NULL) {
> + dprintk("%s NO device for dev_id %s\n",
> __func__, deviceid_fmt(&fl->dev_id));
> - goto out;
> + goto out;
> + }
> }
> if (fl->first_stripe_index < 0 ||
> fl->first_stripe_index > dsaddr->stripe_count) {
> @@ -645,7 +648,7 @@ filelayout_commit(struct pnfs_layout_type *layoutid, int sync,
> stripesz = filelayout_get_stripesize(layoutid);
> dprintk("%s stripesize %Zd\n", __func__, stripesz);
>
> - dsaddr = nfs4_file_layout_dsaddr_get(FILE_MT(data->inode),
> + dsaddr = nfs4_pnfs_device_item_find(FILE_MT(data->inode)->hlist,
> &nfslay->dev_id);
> if (dsaddr == NULL) {
> data->pdata.pnfs_error = -EIO;
> diff --git a/fs/nfs/nfs4filelayout.h b/fs/nfs/nfs4filelayout.h
> index d003809..3821989 100644
> --- a/fs/nfs/nfs4filelayout.h
> +++ b/fs/nfs/nfs4filelayout.h
> @@ -121,7 +121,9 @@ u32 filelayout_dserver_get_index(loff_t offset,
> } while (0)
>
> struct nfs4_file_layout_dsaddr *
> -nfs4_file_layout_dsaddr_get(struct filelayout_mount_type *,
> - struct pnfs_deviceid *);
> +nfs4_pnfs_device_item_find(struct nfs4_pnfs_dev_hlist *hlist,
> + struct pnfs_deviceid *dev_id);
> +struct nfs4_file_layout_dsaddr *
> +get_device_info(struct filelayout_mount_type *mt, struct pnfs_deviceid *dev_id);
>

Andy, this function name is too generic IMO.
better be something more specific to pnfs and file layout.
(I'm committing this patchset as is so we can fix that later)

Benny

> #endif /* FS_NFS_NFS4FILELAYOUT_H */
> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> index e557679..ebb9845 100644
> --- a/fs/nfs/nfs4filelayoutdev.c
> +++ b/fs/nfs/nfs4filelayoutdev.c
> @@ -52,13 +52,6 @@
>
> #define NFSDBG_FACILITY NFSDBG_PNFS_LD
>
> -struct nfs4_file_layout_dsaddr *nfs4_file_layout_dsaddr_get(
> - struct filelayout_mount_type *mt,
> - struct pnfs_deviceid *dev_id);
> -struct nfs4_file_layout_dsaddr *nfs4_pnfs_device_item_find(
> - struct nfs4_pnfs_dev_hlist *hlist,
> - struct pnfs_deviceid *dev_id);
> -
> void
> print_ds_list(struct nfs4_multipath *multipath)
> {
> @@ -665,7 +658,7 @@ decode_and_add_device(struct filelayout_mount_type *mt, struct pnfs_device *dev)
> /* Retrieve the information for dev_id, add it to the list
> * of available devices, and return it.
> */
> -static struct nfs4_file_layout_dsaddr *
> +struct nfs4_file_layout_dsaddr *
> get_device_info(struct filelayout_mount_type *mt,
> struct pnfs_deviceid *dev_id)
> {
> @@ -737,21 +730,6 @@ out_free:
> }
>
> struct nfs4_file_layout_dsaddr *
> -nfs4_file_layout_dsaddr_get(struct filelayout_mount_type *mt,
> - struct pnfs_deviceid *dev_id)
> -{
> - struct nfs4_file_layout_dsaddr *dsaddr;
> -
> - read_lock(&mt->hlist->dev_lock);
> - dsaddr = _device_lookup(mt->hlist, dev_id);
> - read_unlock(&mt->hlist->dev_lock);
> -
> - if (dsaddr == NULL)
> - dsaddr = get_device_info(mt, dev_id);
> - return dsaddr;
> -}
> -
> -struct nfs4_file_layout_dsaddr *
> nfs4_pnfs_device_item_find(struct nfs4_pnfs_dev_hlist *hlist,
> struct pnfs_deviceid *dev_id)
> {
> @@ -798,7 +776,8 @@ nfs4_pnfs_dserver_get(struct pnfs_layout_segment *lseg,
> if (!layout)
> return 1;
>
> - dsaddr = nfs4_file_layout_dsaddr_get(FILE_MT(inode), &layout->dev_id);
> + dsaddr = nfs4_pnfs_device_item_find(FILE_MT(inode)->hlist,
> + &layout->dev_id);
> if (dsaddr == NULL)
> return 1;
>