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

[Pete Wyckoff]

robin.humble+stgt at anu.edu.au wrote on Thu, 07 Feb 2008 21:38 -0500:
> +static int max_queue_cmds_available(struct iscsi_connection *conn)
> +{
> +	int max_queue_cmd;
> +	int max_safe_cmds;
> +
> +	max_safe_cmds = conn->tp->queue_cmds_available(conn);
> +	if (max_safe_cmds < 0) {
> +		eprintf("iSER max_safe_cmds %d < 0\n", max_safe_cmds );
> +		return 0;
> +	}
> +	max_queue_cmd = min( MAX_QUEUE_CMD, max_safe_cmds );
> +
> +	return max_queue_cmd;
> +}

Can we just get rid of MAX_QUEUE_CMD?  You don't need this min(), I
think.  The max_queue_cmd is a property of the transport now, not
iscsi.

> +/*
> + * we know how many iSER connections we have, and also how many areas are
> + * allocated from our mempool, so we can trivially calculate how many
> + * more cmds we can handle without running out of rdma areas.
> + */
> +static int iscsi_rdma_queue_cmds_available(struct iscsi_connection *conn)
> +{
> +	struct conn_info *ci = RDMA_CONN(conn);
> +	struct iser_device *dev = ci->dev;
> +	int cmds;
> +
> +	cmds = (mempool_num - dev->mempool_used)/iser_conn_cnt;
> +
> +	return cmds;
> +}

The divide here is used to spread the remaining available mempool
items on the device across current connections using that device.
This is why I said that iser_conn_cnt should be on dev->.

This is approximate.  For commands that ship no data, we don't need
a mempool buf.  For bidi commands, we need two.  But I think this
approach definitely meets the most common usage model.

Did you think about just doing static allocation?  Since
dev->mempool_used is fairly volatile, you could just give
mempool_num / iser_conn_cnt to each connection.

Probably should make sure you hand out at least 1 command.
I don't know what the spec says about handing out 0.

And yeah, to your future work, may want to add a comment saying how
we could grow the mempool as more clients connect, since there is no
a priori way to know if tgtd will be used for 1 client or lots.

> +static int iscsi_tcp_queue_cmds_available(struct iscsi_connection *conn) {
> +	return MAX_TCP_QUEUE_CMD;
> +}
> +

Sytle, or lack thereof.

		-- Pete