2005-02-11 17:30:43

by Alasdair G Kergon

[permalink] [raw]
Subject: [PATCH] device-mapper: multipath hardware handler for EMC

Outline Hardware Handler for EMC CLARiiON AX/CX-series.

Signed-Off-By: Alasdair G Kergon <[email protected]>
From: Lars Marowsky-Bree <[email protected]>
--- diff/drivers/md/Kconfig 2005-02-09 14:41:52.000000000 +0000
+++ source/drivers/md/Kconfig 2005-02-09 14:42:24.000000000 +0000
@@ -233,5 +233,11 @@
---help---
Allow volume managers to support multipath hardware.

+config DM_MULTIPATH_EMC
+ tristate "EMC CX/AX multipath support (EXPERIMENTAL)"
+ depends on DM_MULTIPATH && BLK_DEV_DM && EXPERIMENTAL
+ ---help---
+ Multipath support for EMC CX/AX series hardware.
+
endmenu

--- diff/drivers/md/Makefile 2005-02-09 14:42:12.000000000 +0000
+++ source/drivers/md/Makefile 2005-02-09 14:42:24.000000000 +0000
@@ -32,6 +32,7 @@
obj-$(CONFIG_BLK_DEV_DM) += dm-mod.o
obj-$(CONFIG_DM_CRYPT) += dm-crypt.o
obj-$(CONFIG_DM_MULTIPATH) += dm-multipath.o dm-round-robin.o
+obj-$(CONFIG_DM_MULTIPATH_EMC) += dm-emc.o
obj-$(CONFIG_DM_SNAPSHOT) += dm-snapshot.o
obj-$(CONFIG_DM_MIRROR) += dm-mirror.o
obj-$(CONFIG_DM_ZERO) += dm-zero.o
--- diff/drivers/md/dm-emc.c 1970-01-01 01:00:00.000000000 +0100
+++ source/drivers/md/dm-emc.c 2005-02-09 14:42:24.000000000 +0000
@@ -0,0 +1,363 @@
+/*
+ * Copyright (C) 2004 SUSE LINUX Products GmbH. All rights reserved.
+ * Copyright (C) 2004 Red Hat, Inc. All rights reserved.
+ *
+ * This file is released under the GPL.
+ *
+ * Multipath support for EMC CLARiiON AX/CX-series hardware.
+ */
+
+#include "dm.h"
+#include "dm-hw-handler.h"
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+
+struct emc_handler {
+ spinlock_t lock;
+
+ /* Whether we should send the short trespass command (FC-series)
+ * or the long version (default for AX/CX CLARiiON arrays). */
+ unsigned short_trespass;
+ /* Whether or not to honor SCSI reservations when initiating a
+ * switch-over. Default: Don't. */
+ unsigned hr;
+
+ unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+};
+
+#define TRESPASS_PAGE 0x22
+#define EMC_FAILOVER_TIMEOUT (60 * HZ)
+
+/* Code borrowed from dm-lsi-rdac by Mike Christie */
+
+static inline void free_bio(struct bio *bio)
+{
+ __free_page(bio->bi_io_vec[0].bv_page);
+ bio_put(bio);
+}
+
+static int emc_endio(struct bio *bio, unsigned int bytes_done, int error)
+{
+ struct path *path = bio->bi_private;
+
+ if (bio->bi_size)
+ return 1;
+
+ /* We also need to look at the sense keys here whether or not to
+ * switch to the next PG etc.
+ *
+ * For now simple logic: either it works or it doesn't.
+ */
+ if (error)
+ dm_pg_init_complete(path, MP_FAIL_PATH);
+ else
+ dm_pg_init_complete(path, 0);
+
+ /* request is freed in block layer */
+ free_bio(bio);
+
+ return 0;
+}
+
+static struct bio *get_failover_bio(struct path *path, unsigned data_size)
+{
+ struct bio *bio;
+ struct page *page;
+
+ bio = bio_alloc(GFP_ATOMIC, 1);
+ if (!bio) {
+ DMERR("dm-emc: get_failover_bio: bio_alloc() failed.");
+ return NULL;
+ }
+
+ bio->bi_rw |= (1 << BIO_RW);
+ 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;
+}
+
+static struct request *get_failover_req(struct emc_handler *h,
+ struct bio *bio, struct path *path)
+{
+ struct request *rq;
+ struct block_device *bdev = bio->bi_bdev;
+ struct request_queue *q = bdev_get_queue(bdev);
+
+ /* FIXME: Figure out why it fails with GFP_ATOMIC. */
+ rq = blk_get_request(q, WRITE, __GFP_WAIT);
+ if (!rq) {
+ DMERR("dm-emc: get_failover_req: blk_get_request failed");
+ return NULL;
+ }
+
+ rq->bio = rq->biotail = bio;
+ blk_rq_bio_prep(q, rq, bio);
+
+ rq->rq_disk = bdev->bd_contains->bd_disk;
+
+ /* bio backed don't set data */
+ rq->buffer = rq->data = NULL;
+ /* rq data_len used for pc cmd's request_bufflen */
+ rq->data_len = bio->bi_size;
+
+ rq->sense = h->sense;
+ memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
+ rq->sense_len = 0;
+
+ memset(&rq->cmd, 0, BLK_MAX_CDB);
+
+ rq->timeout = EMC_FAILOVER_TIMEOUT;
+ rq->flags |= (REQ_BLOCK_PC | REQ_FAILFAST | REQ_NOMERGE);
+
+ return rq;
+}
+
+static struct request *emc_trespass_get(struct emc_handler *h,
+ struct path *path)
+{
+ struct bio *bio;
+ struct request *rq;
+ unsigned char *page22;
+ unsigned char long_trespass_pg[] = {
+ 0, 0, 0, 0,
+ TRESPASS_PAGE, /* Page code */
+ 0x09, /* Page length - 2 */
+ h->hr ? 0x01 : 0x81, /* Trespass code + Honor reservation bit */
+ 0xff, 0xff, /* Trespass target */
+ 0, 0, 0, 0, 0, 0 /* Reserved bytes / unknown */
+ };
+ unsigned char short_trespass_pg[] = {
+ 0, 0, 0, 0,
+ TRESPASS_PAGE, /* Page code */
+ 0x02, /* Page length - 2 */
+ h->hr ? 0x01 : 0x81, /* Trespass code + Honor reservation bit */
+ 0xff, /* Trespass target */
+ };
+ unsigned data_size = h->short_trespass ? sizeof(short_trespass_pg) :
+ sizeof(long_trespass_pg);
+
+ /* get bio backing */
+ if (data_size > PAGE_SIZE)
+ /* this should never happen */
+ return NULL;
+
+ bio = get_failover_bio(path, data_size);
+ if (!bio) {
+ DMERR("dm-emc: emc_trespass_get: no bio");
+ return NULL;
+ }
+
+ page22 = (unsigned char *)bio_data(bio);
+ memset(page22, 0, data_size);
+
+ if (h->short_trespass) {
+ memcpy(page22, short_trespass_pg, data_size);
+ } else {
+ memcpy(page22, long_trespass_pg, data_size);
+ }
+
+ /* get request for block layer packet command */
+ rq = get_failover_req(h, bio, path);
+ if (!rq) {
+ DMERR("dm-emc: emc_trespass_get: no rq");
+ free_bio(bio);
+ return NULL;
+ }
+
+ /* Prepare the command. */
+ rq->cmd[0] = MODE_SELECT;
+ rq->cmd[1] = 0x10;
+ rq->cmd[4] = data_size;
+ rq->cmd_len = COMMAND_SIZE(rq->cmd[0]);
+
+ return rq;
+}
+
+static void emc_pg_init(struct hw_handler *hwh, unsigned bypassed,
+ struct path *path)
+{
+ struct request *rq;
+ struct request_queue *q = bdev_get_queue(path->dev->bdev);
+
+ /*
+ * We can either blindly init the pg (then look at the sense),
+ * or we can send some commands to get the state here (then
+ * possibly send the fo cmnd), or we can also have the
+ * initial state passed into us and then get an update here.
+ */
+ if (!q) {
+ DMINFO("dm-emc: emc_pg_init: no queue");
+ goto fail_path;
+ }
+
+ /* FIXME: The request should be pre-allocated. */
+ rq = emc_trespass_get(hwh->context, path);
+ if (!rq) {
+ DMERR("dm-emc: emc_pg_init: no rq");
+ goto fail_path;
+ }
+
+ DMINFO("dm-emc: emc_pg_init: sending switch-over command");
+ elv_add_request(q, rq, ELEVATOR_INSERT_FRONT, 1);
+ return;
+
+fail_path:
+ dm_pg_init_complete(path, MP_FAIL_PATH);
+}
+
+static struct emc_handler *alloc_emc_handler(void)
+{
+ struct emc_handler *h = kmalloc(sizeof(*h), GFP_KERNEL);
+
+ if (h) {
+ h->lock = SPIN_LOCK_UNLOCKED;
+ }
+
+ return h;
+}
+
+static int emc_ctr(struct hw_handler *hwh, unsigned argc, char **argv)
+{
+ struct emc_handler *h;
+ unsigned hr, short_trespass;
+
+ if (argc == 0) {
+ /* No arguments: use defaults */
+ hr = 0;
+ short_trespass = 0;
+ } else if (argc != 2) {
+ DMWARN("dm-emc hwhandler: incorrect number of arguments");
+ return -EINVAL;
+ } else {
+ if ((sscanf(argv[0], "%u", &short_trespass) != 1)
+ || (short_trespass > 1)) {
+ DMWARN("dm-emc: invalid trespass mode selected");
+ return -EINVAL;
+ }
+
+ if ((sscanf(argv[1], "%u", &hr) != 1)
+ || (hr > 1)) {
+ DMWARN("dm-emc: invalid honor reservation flag selected");
+ return -EINVAL;
+ }
+ }
+
+ h = alloc_emc_handler();
+ if (!h)
+ return -ENOMEM;
+
+ memset(h, 0, sizeof(*h));
+
+ hwh->context = h;
+
+ if ((h->short_trespass = short_trespass))
+ DMWARN("dm-emc: short trespass command will be send");
+ else
+ DMWARN("dm-emc: long trespass command will be send");
+
+ if ((h->hr = hr))
+ DMWARN("dm-emc: honor reservation bit will be set");
+ else
+ DMWARN("dm-emc: honor reservation bit will not be set (default)");
+
+ return 0;
+}
+
+static void emc_dtr(struct hw_handler *hwh)
+{
+ struct emc_handler *h = (struct emc_handler *) hwh->context;
+
+ kfree(h);
+ hwh->context = NULL;
+}
+
+static unsigned emc_err(struct hw_handler *hwh, struct bio *bio)
+{
+ /* FIXME: Patch from axboe still missing */
+#if 0
+ int sense;
+
+ if (bio->bi_error & BIO_SENSE) {
+ sense = bio->bi_error & 0xffffff; /* sense key / asc / ascq */
+
+ if (sense == 0x020403) {
+ /* LUN Not Ready - Manual Intervention Required
+ * indicates this is a passive path.
+ *
+ * FIXME: However, if this is seen and EVPD C0
+ * indicates that this is due to a NDU in
+ * progress, we should set FAIL_PATH too.
+ * This indicates we might have to do a SCSI
+ * inquiry in the end_io path. Ugh. */
+ return MP_BYPASS_PG | MP_RETRY_IO;
+ } else if (sense == 0x052501) {
+ /* An array based copy is in progress. Do not
+ * fail the path, do not bypass to another PG,
+ * do not retry. Fail the IO immediately.
+ * (Actually this is the same conclusion as in
+ * the default handler, but lets make sure.) */
+ return 0;
+ } else if (sense == 0x062900) {
+ /* Unit Attention Code. This is the first IO
+ * to the new path, so just retry. */
+ return MP_RETRY_IO;
+ }
+ }
+#endif
+
+ /* Try default handler */
+ return dm_scsi_err_handler(hwh, bio);
+}
+
+static struct hw_handler_type emc_hwh = {
+ .name = "emc",
+ .module = THIS_MODULE,
+ .ctr = emc_ctr,
+ .dtr = emc_dtr,
+ .pg_init = emc_pg_init,
+ .err = emc_err,
+};
+
+static int __init dm_emc_init(void)
+{
+ int r = dm_register_hw_handler(&emc_hwh);
+
+ if (r < 0)
+ DMERR("emc: register failed %d", r);
+
+ DMINFO("dm-emc version 0.0.3 loaded");
+
+ return r;
+}
+
+static void __exit dm_emc_exit(void)
+{
+ int r = dm_unregister_hw_handler(&emc_hwh);
+
+ if (r < 0)
+ DMERR("emc: unregister failed %d", r);
+}
+
+module_init(dm_emc_init);
+module_exit(dm_emc_exit);
+
+MODULE_DESCRIPTION(DM_NAME " EMC CX/AX/FC-family multipath");
+MODULE_AUTHOR("Lars Marowsky-Bree <[email protected]>");
+MODULE_LICENSE("GPL");


2005-02-11 19:59:03

by Christoph Hellwig

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

> +/* 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.

2005-02-12 04:08:56

by Lars Marowsky-Bree

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

On 2005-02-11T19:58:41, Christoph Hellwig <[email protected]> wrote:

> > +/* Code borrowed from dm-lsi-rdac by Mike Christie */
>
> Any reason that module isn't submitted?

No idea why.

> > + 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.

OK.

> > + 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);
>
> ?

Yes, I first did some other things there than just copying the commands
around, it can surely benefit from cleanup.

> > +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);

Came in via the copy, good catch.

> > +static unsigned emc_err(struct hw_handler *hwh, struct bio *bio)
> > +{
> > + /* FIXME: Patch from axboe still missing */
>
> it's in -mm now afaik??

No, it's not. That's the request sense keys, but here we're dealing with
the bio.

> > +#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).

I'll go look them up.

> Also please try to use constants instead of magic numbers.

Noted. I'll clean this part up when I actually have sense keys to try,
so far this was mostly about getting that tiny bit of logic in.


Sincerely,
Lars Marowsky-Br?e <[email protected]>

--
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business

2005-02-12 22:08:45

by Mike Christie

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

Christoph Hellwig wrote:
>>+/* Code borrowed from dm-lsi-rdac by Mike Christie */
>
>
> Any reason that module isn't submitted?
>

I do not have access to their HW specs, and have been busy with
some iscsi things so I did not have time to finish things up.

I was also hoping LSI would soon figure out that they should be
using dm (I have seen some of them pop up now and then only)
and take over the work.