On Fri, Apr 24, 2009 at 05:08:02PM +0900, Kiyoshi Ueda wrote:
> + struct selector *s = (struct selector *) ps->context;
> + status_type_t type, char *result, unsigned int maxlen)
> + int sz = 0;
> + DMEMIT("%u %lu ", atomic_read(&pi->in_flight_size),
(similar comments)
> + DMEMIT("%u %lu ", pi->repeat_count, pi->perf);
Need to handle both 32 and 64 bit archs: cast to llu.
> + if ((argc == 2) && (sscanf(argv[1], "%lu", &perf) != 1)) {
Please do sscanf for size_t the same way as dm-crypt does.
(Or scan for lu, if the intent is to impose a size limit ahead of the later
do_div() then cast explicitly.)
> +/*
> + * Returns:
> + * < 0 : pi1 is better
> + * 0 : no difference between pi1 and pi2
> + * > 0 : pi2 is better
> + */
Please document the parameters and algorithm.
Nothing explains what the performance value is for and examples of anticipated values.
> + do_div(sz1, pi1->perf);
> + do_div(sz2, pi2->perf);
Or sector_div ?
But what's the performance of those divisions like?
Is there a better way of getting the same answer?
Is there validation limiting the size of 'perf'?
(Also, did you check there's adequate locking & memory barriers around all the
atomics?)
Alasdair
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
