Re: [PATCH] device-mapper: multipath hardware handler for EMC

Christoph Hellwig
2005-02-11T19:59:03Z

> +/* Code borrowed from dm-lsi-rdac by Mike Christie */

Any reason that module isn't submitted?

> + bio->bi_bdev = path->dev->bdev;
> + bio->bi_sector = 0;
> + bio->bi_private = path;
> + bio->bi_end_io = emc_endio;
> +
> + page = alloc_page(GFP_ATOMIC);
> + if (!page) {
> + DMERR("dm-emc: get_failover_bio: alloc_page() failed.");
> + bio_put(bio);
> + return NULL;
> + }
> +
> + if (bio_add_page(bio, page, data_size, 0) != data_size) {
> + DMERR("dm-emc: get_failover_bio: alloc_page() failed.");
> + __free_page(page);
> + bio_put(bio);
> + return NULL;
> + }
> +
> + return bio;

this would benefit from goto unwinding.

> + if (h->short_trespass) {
> + memcpy(page22, short_trespass_pg, data_size);
> + } else {
> + memcpy(page22, long_trespass_pg, data_size);
> + }

memcpy(page22, h->short_trespass ?
short_trespass_pg : long_trespass_pg, data_size);

?

> +static struct emc_handler *alloc_emc_handler(void)
> +{
> + struct emc_handler *h = kmalloc(sizeof(*h), GFP_KERNEL);
> +
> + if (h) {
> + h->lock = SPIN_LOCK_UNLOCKED;
> + }

if (h)
spin_lock_init(&h->lock);

> +static unsigned emc_err(struct hw_handler *hwh, struct bio *bio)
> +{
> + /* FIXME: Patch from axboe still missing */

it's in -mm now afaik??

> +#if 0
> + int sense;
> +
> + if (bio->bi_error & BIO_SENSE) {
> + sense = bio->bi_error & 0xffffff; /* sense key / asc / ascq */
> +
> + if (sense == 0x020403) {

please use the sense handling helpers from Doug Gilbert so you can handle
the descriptor sense format aswell. (And make the code a lot clear).

Also please try to use constants instead of magic numbers.