Re: [RFC PATCH 1/2] NFSv4.1: Convert slotid from u8 to u32

[Benny Halevy <bhalevy@tonian.com>]

I reviewed both patches and they look good to me.

Benny

On 2012-02-07 02:57, Trond Myklebust wrote:
> It is perfectly legal to negotiate up to 2^32-1 slots in the protocol,
> and with 10GigE, we are already seeing that 255 slots is far too limiting.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/callback.h         |    2 +-
>  fs/nfs/callback_xdr.c     |    6 +++---
>  fs/nfs/nfs4proc.c         |   21 ++++++++++-----------
>  include/linux/nfs_fs_sb.h |    7 ++++---
>  include/linux/nfs_xdr.h   |    2 +-
>  5 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
> index 197e0d3..a5527c9 100644
> --- a/fs/nfs/callback.h
> +++ b/fs/nfs/callback.h
> @@ -38,7 +38,7 @@ enum nfs4_callback_opnum {
>  struct cb_process_state {
>  	__be32			drc_status;
>  	struct nfs_client	*clp;
> -	int			slotid;
> +	u32			slotid;
>  	struct net		*net;
>  };
>  
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index 2e37224..5466829 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -759,14 +759,14 @@ static void nfs4_callback_free_slot(struct nfs4_session *session)
>  	 * Let the state manager know callback processing done.
>  	 * A single slot, so highest used slotid is either 0 or -1
>  	 */
> -	tbl->highest_used_slotid = -1;
> +	tbl->highest_used_slotid = NFS4_NO_SLOT;
>  	nfs4_check_drain_bc_complete(session);
>  	spin_unlock(&tbl->slot_tbl_lock);
>  }
>  
>  static void nfs4_cb_free_slot(struct cb_process_state *cps)
>  {
> -	if (cps->slotid != -1)
> +	if (cps->slotid != NFS4_NO_SLOT)
>  		nfs4_callback_free_slot(cps->clp->cl_session);
>  }
>  
> @@ -860,7 +860,7 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
>  	struct cb_process_state cps = {
>  		.drc_status = 0,
>  		.clp = NULL,
> -		.slotid = -1,
> +		.slotid = NFS4_NO_SLOT,
>  		.net = rqstp->rq_xprt->xpt_net,
>  	};
>  	unsigned int nops = 0;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 482ed97..0dc5dfb 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -402,7 +402,7 @@ static void nfs4_check_drain_fc_complete(struct nfs4_session *ses)
>  		return;
>  	}
>  
> -	if (ses->fc_slot_table.highest_used_slotid != -1)
> +	if (ses->fc_slot_table.highest_used_slotid != NFS4_NO_SLOT)
>  		return;
>  
>  	dprintk("%s COMPLETE: Session Fore Channel Drained\n", __func__);
> @@ -415,7 +415,7 @@ static void nfs4_check_drain_fc_complete(struct nfs4_session *ses)
>  void nfs4_check_drain_bc_complete(struct nfs4_session *ses)
>  {
>  	if (!test_bit(NFS4_SESSION_DRAINING, &ses->session_state) ||
> -	    ses->bc_slot_table.highest_used_slotid != -1)
> +	    ses->bc_slot_table.highest_used_slotid != NFS4_NO_SLOT)
>  		return;
>  	dprintk("%s COMPLETE: Session Back Channel Drained\n", __func__);
>  	complete(&ses->bc_slot_table.complete);
> @@ -514,14 +514,13 @@ static int nfs4_sequence_done(struct rpc_task *task,
>   *
>   * Note: must be called with under the slot_tbl_lock.
>   */
> -static u8
> +static u32
>  nfs4_find_slot(struct nfs4_slot_table *tbl)
>  {
> -	int slotid;
> -	u8 ret_id = NFS4_MAX_SLOT_TABLE;
> -	BUILD_BUG_ON((u8)NFS4_MAX_SLOT_TABLE != (int)NFS4_MAX_SLOT_TABLE);
> +	u32 slotid;
> +	u32 ret_id = NFS4_NO_SLOT;
>  
> -	dprintk("--> %s used_slots=%04lx highest_used=%d max_slots=%d\n",
> +	dprintk("--> %s used_slots=%04lx highest_used=%u max_slots=%u\n",
>  		__func__, tbl->used_slots[0], tbl->highest_used_slotid,
>  		tbl->max_slots);
>  	slotid = find_first_zero_bit(tbl->used_slots, tbl->max_slots);
> @@ -555,7 +554,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
>  {
>  	struct nfs4_slot *slot;
>  	struct nfs4_slot_table *tbl;
> -	u8 slotid;
> +	u32 slotid;
>  
>  	dprintk("--> %s\n", __func__);
>  	/* slot already allocated? */
> @@ -5144,7 +5143,7 @@ static int nfs4_init_slot_table(struct nfs4_slot_table *tbl,
>  	spin_lock(&tbl->slot_tbl_lock);
>  	tbl->max_slots = max_slots;
>  	tbl->slots = slot;
> -	tbl->highest_used_slotid = -1;  /* no slot is currently used */
> +	tbl->highest_used_slotid = NFS4_NO_SLOT;  /* no slot is currently used */
>  	spin_unlock(&tbl->slot_tbl_lock);
>  	dprintk("%s: tbl=%p slots=%p max_slots=%d\n", __func__,
>  		tbl, tbl->slots, tbl->max_slots);
> @@ -5196,13 +5195,13 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
>  		return NULL;
>  
>  	tbl = &session->fc_slot_table;
> -	tbl->highest_used_slotid = -1;
> +	tbl->highest_used_slotid = NFS4_NO_SLOT;
>  	spin_lock_init(&tbl->slot_tbl_lock);
>  	rpc_init_priority_wait_queue(&tbl->slot_tbl_waitq, "ForeChannel Slot table");
>  	init_completion(&tbl->complete);
>  
>  	tbl = &session->bc_slot_table;
> -	tbl->highest_used_slotid = -1;
> +	tbl->highest_used_slotid = NFS4_NO_SLOT;
>  	spin_lock_init(&tbl->slot_tbl_lock);
>  	rpc_init_wait_queue(&tbl->slot_tbl_waitq, "BackChannel Slot table");
>  	init_completion(&tbl->complete);
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 9e101c1..2ae57f2 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -191,6 +191,7 @@ struct nfs_server {
>  
>  /* maximum number of slots to use */
>  #define NFS4_MAX_SLOT_TABLE (128U)
> +#define NFS4_NO_SLOT ((u32)-1)
>  
>  #if defined(CONFIG_NFS_V4)
>  
> @@ -201,10 +202,10 @@ struct nfs4_slot_table {
>  	unsigned long   used_slots[SLOT_TABLE_SZ]; /* used/unused bitmap */
>  	spinlock_t	slot_tbl_lock;
>  	struct rpc_wait_queue	slot_tbl_waitq;	/* allocators may wait here */
> -	int		max_slots;		/* # slots in table */
> -	int		highest_used_slotid;	/* sent to server on each SEQ.
> +	u32		max_slots;		/* # slots in table */
> +	u32		highest_used_slotid;	/* sent to server on each SEQ.
>  						 * op for dynamic resizing */
> -	int		target_max_slots;	/* Set by CB_RECALL_SLOT as
> +	u32		target_max_slots;	/* Set by CB_RECALL_SLOT as
>  						 * the new max_slots */
>  	struct completion complete;
>  };
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 144419a..adbc84a 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -181,7 +181,7 @@ struct nfs4_slot {
>  
>  struct nfs4_sequence_args {
>  	struct nfs4_session	*sa_session;
> -	u8			sa_slotid;
> +	u32			sa_slotid;
>  	u8			sa_cache_this;
>  };
>