Re: [PATCH 2/3] dm-mpath: add queue-length oriented dynamic load balancer

[Alasdair G Kergon <agk@redhat.com>]

On Fri, Apr 24, 2009 at 05:07:26PM +0900, Kiyoshi Ueda wrote:
> +	unsigned int		repeat_count;

Just 'unsigned repeat_count'

> +	struct selector *s = (struct selector *) ps->context;

Drop the cast when it's from void.

	struct selector *s = ps->context;

> +static int ql_status(struct path_selector *ps, struct dm_path *path,
> +		     status_type_t type, char *result, unsigned int maxlen)

unsigned maxlen

(We're doing things like this in all new patches, but only changing existing
code when it's touched for some other reason.)

> +	int sz = 0;

signed or unsigned sz?
(It's already used inconsistently, I know - but is unsigned better here?)

> +		case STATUSTYPE_INFO:
> +			DMEMIT("%u ", atomic_read(&pi->qlen));

Signed or unsigned?

> +	/* Parse the arguments */

Please document parameters and selection algorithm in Documentation dir and
maybe in inline comments too - it's not immediately obvious exactly how it
behaves.

> +static struct dm_path *ql_select_path(struct path_selector *ps,
> +				      unsigned *repeat_count, size_t nr_bytes)

Alasdair