[pnfs] [PATCH 18/28] pnfsblock: ask for layout_blksize on mount

Benny Halevy bhalevy at panasas.com
Thu Mar 13 09:57:25 EDT 2008


On Mar. 11, 2008, 21:31 +0200, Fred Isaman <iisaman at citi.umich.edu> wrote:
> Note this requires fattr bitmap to be length 3, instead of 2.
> This patch just hacks in enough to get by.  Really need to
> separate out a patch that correctly handles more generic bitmap
> lengths.
> 
> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
> ---
>  fs/nfs/client.c           |    1 +
>  fs/nfs/nfs4_fs.h          |    2 +-
>  fs/nfs/nfs4proc.c         |    5 ++-
>  fs/nfs/nfs4xdr.c          |   62 +++++++++++++++++++++++++++++++++++++++------
>  include/linux/nfs4.h      |    1 +
>  include/linux/nfs_fs_sb.h |    1 +
>  include/linux/nfs_xdr.h   |    3 +-
>  7 files changed, 63 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index c012919..9f71b98 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -645,6 +645,7 @@ static void nfs_server_set_fsinfo(struct nfs_server *server, struct nfs_fsinfo *
>  		server->wsize = NFS_MAX_FILE_IO_SIZE;
>  	server->wpages = (server->wsize + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
>  #ifdef CONFIG_PNFS
> +	server->pnfs_blksize = fsinfo->blksize; /* XXX Need default if 0? */
>  	/* Save the layout type for use during init of layout driver */
>  	server->pnfs_fs_ltype = fsinfo->layoutclass;
>  #endif /* CONFIG_PNFS */
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 0da41a7..a2020e3 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -219,7 +219,7 @@ extern int nfs4_proc_destroy_session(struct nfs4_session *, struct rpc_clnt *);
>  extern const u32 nfs4_fattr_bitmap[2];
>  extern const u32 nfs4_statfs_bitmap[2];
>  extern const u32 nfs4_pathconf_bitmap[2];
> -extern const u32 nfs4_fsinfo_bitmap[2];
> +extern const u32 nfs4_fsinfo_bitmap[3];
>  extern const u32 nfs4_fs_locations_bitmap[2];
>  
>  /* nfs4renewd.c */
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index b20c07a..021e10b 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -125,12 +125,13 @@ const u32 nfs4_pathconf_bitmap[2] = {
>  	0
>  };
>  
> -const u32 nfs4_fsinfo_bitmap[2] = { FATTR4_WORD0_MAXFILESIZE
> +const u32 nfs4_fsinfo_bitmap[3] = { FATTR4_WORD0_MAXFILESIZE
>  			| FATTR4_WORD0_MAXREAD
>  			| FATTR4_WORD0_MAXWRITE
>  			| FATTR4_WORD0_LEASE_TIME,
>  #ifdef CONFIG_PNFS
> -			FATTR4_WORD1_FS_LAYOUT_TYPES
> +			FATTR4_WORD1_FS_LAYOUT_TYPES,
> +			FATTR4_WORD2_LAYOUT_BLKSIZE
>  #else /* CONFIG_PNFS */
>  			0
>  #endif /* CONFIG_PNFS */
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 8edf687..b4ff9cc 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -91,7 +91,7 @@ static int nfs4_stat_to_errno(int);
>  #define encode_getfh_maxsz      (op_encode_hdr_maxsz)
>  #define decode_getfh_maxsz      (op_decode_hdr_maxsz + 1 + \
>  				((3+NFS4_FHSIZE) >> 2))
> -#define nfs4_fattr_bitmap_maxsz 3
> +#define nfs4_fattr_bitmap_maxsz 4
>  #define encode_getattr_maxsz    (op_encode_hdr_maxsz + nfs4_fattr_bitmap_maxsz)
>  #define nfs4_name_maxsz		(1 + ((3 + NFS4_MAXNAMLEN) >> 2))
>  #define nfs4_path_maxsz		(1 + ((3 + NFS4_MAXPATHLEN) >> 2))
> @@ -113,7 +113,12 @@ static int nfs4_stat_to_errno(int);
>  #define encode_restorefh_maxsz  (op_encode_hdr_maxsz)
>  #define decode_restorefh_maxsz  (op_decode_hdr_maxsz)
>  #define encode_fsinfo_maxsz	(encode_getattr_maxsz)
> +#if !defined(CONFIG_PNFS)
>  #define decode_fsinfo_maxsz	(op_decode_hdr_maxsz + 11)
> +#else /* CONFIG_PNFS */
> +/* The size 15 assumes only a single layoutdriver is returned. */
> +#define decode_fsinfo_maxsz	(op_decode_hdr_maxsz + 15)
> +#endif /* CONFIG_PNFS */
>  #define encode_renew_maxsz	(op_encode_hdr_maxsz + 3)
>  #define decode_renew_maxsz	(op_decode_hdr_maxsz)
>  #define encode_setclientid_maxsz \
> @@ -1077,6 +1082,20 @@ static int encode_getattr_two(struct xdr_stream *xdr, uint32_t bm0, uint32_t bm1
>          return 0;
>  }
>  
> +static int encode_getattr_three(struct xdr_stream *xdr,
> +				uint32_t bm0, uint32_t bm1, uint32_t bm2)
> +{
> +	__be32 *p;
> +
> +	RESERVE_SPACE(20);
> +	WRITE32(OP_GETATTR);
> +	WRITE32(3);
> +	WRITE32(bm0);
> +	WRITE32(bm1);
> +	WRITE32(bm2);

It'd be worth optimizing for the non-pnfs case
by checking if bm1 or bm2 are not zero, e.g.:

	RESERVE_SPACE(4);
	WRITE32(OP_GETATTR);
	if (unlikely(bm2)) {
		RESERVE_SPACE(16);
		WRITE32(3);
		WRITE32(bm0);
		WRITE32(bm1);
		WRITE32(bm2);
	} else if (unlikely(bm1)) {
		RESERVE_SPACE(12);
		WRITE32(2);
		WRITE32(bm0);
		WRITE32(bm1);
	} else {
		RESERVE_SPACE(8);
		WRITE32(1);
		WRITE32(bm0);
	}

> +	return 0;
> +}
> +
>  static int encode_getfattr(struct xdr_stream *xdr, const u32* bitmask)
>  {
>  	return encode_getattr_two(xdr,
> @@ -1086,8 +1105,10 @@ static int encode_getfattr(struct xdr_stream *xdr, const u32* bitmask)
>  
>  static int encode_fsinfo(struct xdr_stream *xdr, const u32* bitmask)
>  {
> -	return encode_getattr_two(xdr, bitmask[0] & nfs4_fsinfo_bitmap[0],
> -			bitmask[1] & nfs4_fsinfo_bitmap[1]);
> +	return encode_getattr_three(xdr,
> +			bitmask[0] & nfs4_fsinfo_bitmap[0],
> +			bitmask[1] & nfs4_fsinfo_bitmap[1],
> +			bitmask[2] & nfs4_fsinfo_bitmap[2]);
>  }
>  
>  #ifdef CONFIG_PNFS
> @@ -3726,12 +3747,15 @@ static int decode_attr_bitmap(struct xdr_stream *xdr, uint32_t *bitmap)
>  	READ_BUF(4);
>  	READ32(bmlen);
>  
> -	bitmap[0] = bitmap[1] = 0;
> +	bitmap[0] = bitmap[1] = bitmap[2] = 0;
>  	READ_BUF((bmlen << 2));
>  	if (bmlen > 0) {
>  		READ32(bitmap[0]);
> -		if (bmlen > 1)
> +		if (bmlen > 1) {
>  			READ32(bitmap[1]);
> +			if (bmlen > 2)
> +				READ32(bitmap[2]);
> +		}
>  	}
>  	return 0;
>  }
> @@ -3752,8 +3776,9 @@ static int decode_attr_supported(struct xdr_stream *xdr, uint32_t *bitmap, uint3
>  		decode_attr_bitmap(xdr, bitmask);
>  		bitmap[0] &= ~FATTR4_WORD0_SUPPORTED_ATTRS;
>  	} else
> -		bitmask[0] = bitmask[1] = 0;
> -	dprintk("%s: bitmask=0x%x%x\n", __FUNCTION__, bitmask[0], bitmask[1]);
> +		bitmask[0] = bitmask[1] = bitmask[2] = 0;
> +	dprintk("%s: bitmask=0x%x%x%x\n", __FUNCTION__,

that format string is problematic since it's too long, and 
the %x's aren't zero padded.  Let's use %08x:%08x:%08x instead.

> +				bitmask[0], bitmask[1], bitmask[2]);
>  	return 0;
>  }
>  
> @@ -4480,6 +4505,24 @@ static int decode_attr_pnfstype(struct xdr_stream *xdr, uint32_t *bitmap, uint32
>  }
>  
>  /*
> + * The prefered block size for layout directed io
> + */
> +static int decode_attr_layout_blksize(struct xdr_stream *xdr, uint32_t *bitmap,
> +				      uint32_t *res)
> +{
> +	__be32 *p;
> +
> +	dprintk("bitmap is %x\n", bitmap[2]);

please print also the function name
	dprintk("%s: bitmap is %08x\n", __func__, bitmap[2]);

(and while you're at it, the dprintk in decode_attr_pnfstype needs to 
be fixed similarly)

> +	*res = 0;
> +	if (likely(bitmap[2] & FATTR4_WORD2_LAYOUT_BLKSIZE)) {
> +		READ_BUF(4);
> +		READ32(*res);
> +		bitmap[2] &= ~FATTR4_WORD2_LAYOUT_BLKSIZE;
> +	}
> +	return 0;
> +}
> +
> +/*
>   * Decode LAYOUTCOMMIT reply
>   */
>  static int decode_pnfs_layoutcommit(struct xdr_stream *xdr,
> @@ -4733,7 +4776,7 @@ xdr_error:
>  static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
>  {
>  	__be32 *savep;
> -	uint32_t attrlen, bitmap[2];
> +	uint32_t attrlen, bitmap[3];
>  	int status;
>  
>  	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
> @@ -4759,6 +4802,9 @@ static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
>  	status = decode_attr_pnfstype(xdr, bitmap, &fsinfo->layoutclass);
>  	if (status)
>  		goto xdr_error;
> +	status = decode_attr_layout_blksize(xdr, bitmap, &fsinfo->blksize);
> +	if (status)
> +		goto xdr_error;
>  #endif /* CONFIG_PNFS */
>  
>  	status = verify_attr_len(xdr, savep, attrlen);
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 20be136..a58bd36 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -420,6 +420,7 @@ enum lock_type4 {
>  #define FATTR4_WORD1_TIME_MODIFY_SET    (1UL << 22)
>  #define FATTR4_WORD1_MOUNTED_ON_FILEID  (1UL << 23)
>  #define FATTR4_WORD1_FS_LAYOUT_TYPES	(1UL << 30)
> +#define FATTR4_WORD2_LAYOUT_BLKSIZE	(1UL << 1)

sigh, I wish we had an enum with the absolute attribute IDs
copied from the .x file, and then this could look like
#define FATTR4_WORD2_LAYOUT_BLKSIZE	(1UL << (FATTR4_LAYOUT_BLKSIZE & 31))

>  
>  #define NFSPROC4_NULL 0
>  #define NFSPROC4_COMPOUND 1
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 5199801..1a3deca 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -134,6 +134,7 @@ struct nfs_server {
>  
>  #ifdef CONFIG_PNFS
>  	u32				pnfs_fs_ltype;/* fs_layouttype attr */
> +	u32				pnfs_blksize; /* layout_blksize attr */
>  	struct pnfs_layoutdriver_type  *pnfs_curr_ld; /* Active layout driver */
>  	struct pnfs_mount_type         *pnfs_mountid; /* Mount identifier for
>  							 layout driver */
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index d991743..af69596 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -55,7 +55,7 @@ struct nfs_fattr {
>  	struct timespec		atime;
>  	struct timespec		mtime;
>  	struct timespec		ctime;
> -	__u32			bitmap[2];	/* NFSv4 returned attribute bitmap */
> +	__u32			bitmap[3];	/* NFSv4 returned attr bitmap */
>  	__u64			change_attr;	/* NFSv4 change attribute */
>  	__u64			pre_change_attr;/* pre-op NFSv4 change attribute */
>  	unsigned long		time_start;
> @@ -84,6 +84,7 @@ struct nfs_fsinfo {
>  	__u32			lease_time; /* in seconds */
>  #if defined(CONFIG_PNFS)
>  	__u32			layoutclass; /* supported pnfs layout driver */
> +	__u32			blksize; /* preferred pnfs io block size */
>  #endif
>  };
>  



More information about the pNFS mailing list