[Stgt-devel] [PATCH 1/2] iSER throttling

[Pete Wyckoff]

robin.humble+stgt at anu.edu.au wrote on Thu, 07 Feb 2008 21:36 -0500:
> with a few iSER writers it's easy to run out of RDMA areas in the tgtd
> mempool. currently tgtd gives up in this case, leading to a mess.
> these 2 patches throttle iSER requests so that there are always enough
> RDMA areas left in the mempool left to do i/o.
> 
> it's been tested with 1 to 16 initiators and it seems to work.
> 
> funtionality of the iSER code is unchanged when there's 1 initiator and
> 1 target.
> functionality of the tcp transport code is unchanged, although these
> patches add enough framework that the tcp code could have throttling
> enabled sometime in the future.
> 
> possible extensions to this work are
>  - to tell the user when they are frequently running low on RDMA areas
>    and suggest that they recompile with a higher number
>  - to make the RDMA mempool size a runtime configurable so that it can
>    be increased without a recompile
>  - to add tcp accounting and throttling

Brilliant stuff.  This is definitely a worthwhile improvement.  Some
comments.

> diff -ruN ../tgt/usr/iscsi/iscsi_rdma.c ./usr/iscsi/iscsi_rdma.c
> --- ../tgt/usr/iscsi/iscsi_rdma.c	2008-01-23 12:50:27.000000000 +1100
> +++ ./usr/iscsi/iscsi_rdma.c	2008-01-24 18:22:32.000000000 +1100
> @@ -196,6 +196,9 @@
>  
>  	/* free and allocated mempool entries */
>  	struct list_head mempool_free, mempool_alloc;
> +
> +        /* rdma mempool accounting */
> +        int mempool_used;
>  };

Tomo will yell at you for not running checkpatch.  Those spaces
should be tabs.

> @@ -213,6 +216,7 @@
>  
>  /* all iser connections */
>  static struct list_head iser_conn_list;
> +static int iser_conn_cnt;
>  
>  /* if any task needs an rdma read or write slot to proceed */
>  static int waiting_rdma_slot;
> @@ -555,6 +559,7 @@
>  	dev->mempool_listbuf = listbuf;
>  	INIT_LIST_HEAD(&dev->mempool_free);
>  	INIT_LIST_HEAD(&dev->mempool_alloc);
> +	dev->mempool_used = 0;
>  
>  	for (i = 0; i < mempool_num; i++) {
>  		mp = (void *) listbuf;
> @@ -793,6 +798,7 @@
>  	dprintf("established conn %p\n", ci);
>  	list_del(&ci->iser_conn_list);
>  	list_add(&ci->iser_conn_list, &iser_conn_list);
> +	iser_conn_cnt++;
>  }

I think iser_conn_cnt should be a property on the dev, not global.
Since we allocate mempools per device.

Also might as well fold these two patches into one, since it is hard
to understand the role of iser_conn_cnt without the uses of it in
patch 2.

		-- Pete