Skip to contentSkip to conversation

dm-hp-sw: HP Active/Passive hardware handler baseline

PATCH In review Dave Wysochanski 2 participants 3 messages May 23, 2007 – May 24, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dm-hp-sw: HP Active/Passive hardware handler baseline

Baseline support for HP Active / Passive arrays. Possible enhancements include handling specific check conditions, pg_init retries, and configurable timeouts.

--

drivers/md/Makefile +1 -0
Index: linux-2.6.22-rc1/drivers/md/Makefile
===================================================================
@@ -34,6 +34,7 @@ obj-$(CONFIG_DM_CRYPT)		+= dm-crypt.o
 obj-$(CONFIG_DM_DELAY)		+= dm-delay.o
 obj-$(CONFIG_DM_MULTIPATH)	+= dm-multipath.o dm-round-robin.o
 obj-$(CONFIG_DM_MULTIPATH_EMC)	+= dm-emc.o
+obj-$(CONFIG_DM_MULTIPATH_HP)	+= dm-hp-sw.o
 obj-$(CONFIG_DM_SNAPSHOT)	+= dm-snapshot.o
 obj-$(CONFIG_DM_MIRROR)		+= dm-mirror.o
 obj-$(CONFIG_DM_ZERO)		+= dm-zero.o
drivers/md/dm-hp-sw.c new file +211 -0

211 lines not on this page · raw message

drivers/md/Kconfig +6 -0
Index: linux-2.6.22-rc1/drivers/md/Kconfig
===================================================================
@@ -271,6 +271,12 @@ config DM_DELAY
 
 	If unsure, say N.
 
+config DM_MULTIPATH_HP
+        tristate "HP MSA multipath support (EXPERIMENTAL)"
+        depends on DM_MULTIPATH && BLK_DEV_DM && EXPERIMENTAL
+        ---help---
+          Multipath support for HP MSA (Active/Passive) series hardware.
+
 endmenu
 
 endif
#1 lore raw

drivers/md/dm-hp-sw.c line 23

Quoted 8 lines · Dave Wysochanski
drivers/md/dm-hp-sw.c new file +8 -0
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_transport_fc.h>
+#include <linux/list.h>
+#include <linux/types.h>

you should do linux includes then scsi ones. Do you even need the fc, host or cmnd includes?

Quoted 25 lines
drivers/md/dm-hp-sw.c new file +25 -0
+
+#include "dm.h"
+#include "dm-hw-handler.h"
+
+#define DM_MSG_PREFIX "multipath hp"
+
+struct hp_sw_context {
+	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+};
+
+
+/**
+ * hp_sw_end_io - Completion handler for HP path activation.
+ * @req: failover request
+ * @error: scsi-ml error
+ *
+ *  Check sense data, free request structure, and notify dm that
+ *  pg initialization has completed.
+ *
+ * Context: scsi-ml softirq
+ *
+ * Possible optimizations
+ * 1. Actually check sense data for retryable error (e.g. NOT_READY)
+ */
+static void hp_sw_end_io(struct request *req, int error)

drivers/md/dm-hp-sw.c line 58

Quoted 10 lines
drivers/md/dm-hp-sw.c new file +10 -0
+{
+	struct dm_path *path = req->end_io_data;
+	unsigned err_flags;
+
+	if (!error) {
+		err_flags = 0;
+		DMDEBUG("hp_sw: path activation command on %s - success",
+		       	path->dev->name);
+	}
+	else {

else should be on the same line as "{"

I also think we should handle the NOT_READY case that was reported before.

Quoted 70 lines
drivers/md/dm-hp-sw.c new file +70 -0
+		DMWARN("hp_sw: path activation command on %s - error=0x%x",
+		       path->dev->name, error);
+		err_flags = MP_FAIL_PATH;
+	}
+
+	req->end_io_data = NULL;
+	__blk_put_request(req->q, req);
+	dm_pg_init_complete(path, err_flags);
+}
+
+/**
+ * hp_sw_get_request - Allocate an HP specific path activation request
+ * @path: path on which request will be sent (needed for request queue)
+ *
+ * The START command is used for path activation request.
+ * These arrays are controller-based failover, not LUN based.
+ * One START command issued to a single path will fail over all
+ * LUNs for the same controller.
+ *
+ * Possible optimizations
+ * 1. Make timeout configurable
+ * 2. Preallocate request
+ */
+static struct request *hp_sw_get_request(struct dm_path *path)
+{
+	struct request *req=NULL;
+	struct block_device *bdev = path->dev->bdev;
+	struct request_queue *q = bdev_get_queue(bdev);
+	struct hp_sw_context *h = path->hwhcontext;
+
+	req = blk_get_request(q, WRITE, GFP_ATOMIC);
+	if (req == NULL)
+		goto exit;
+
+	req->timeout = 60*HZ;
+
+	req->errors = 0;
+	req->cmd_type = REQ_TYPE_BLOCK_PC;
+	req->cmd_flags |= REQ_FAILFAST | REQ_NOMERGE;
+	req->end_io = hp_sw_end_io;
+	req->end_io_data = path;
+	req->sense = h->sense;
+	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
+
+	memset(&req->cmd, 0, BLK_MAX_CDB);
+	req->cmd[0] = START_STOP;
+	req->cmd[4] = 1;
+	req->cmd_len = COMMAND_SIZE(req->cmd[0]);
+exit:
+	return req;
+}
+
+/**
+ * hp_sw_pg_init - HP path activation implementation.
+ * @hwh: hardware handler specific data
+ * @bypassed: unused; is the path group bypassed? (see dm-mpath.c)
+ * @path: path to send initialization command
+ *
+ * Send an HP-specific path activation command on 'path'.
+ * Do not try to optimize in any way, just send the activation command.
+ * More than one path activation command may be sent to the same controller.
+ * This seems to work fine for basic failover support.
+ *
+ * Possible optimizations
+ * 1. Detect an in-progress activation request and avoid submitting another one
+ * 2. Model the controller and only send a single activation request at a time
+ * 3. Determine the state of a path before sending an activation request
+ *
+ * Context: kmpathd (see process_queued_ios() in dm-mpath.c)
+ */

drivers/md/dm-hp-sw.c line 138

Quoted 10 lines
drivers/md/dm-hp-sw.c new file +10 -0
+static void hp_sw_pg_init(struct hw_handler *hwh, unsigned bypassed,
+			  struct dm_path *path)
+{
+	struct request *req;
+	struct hp_sw_context *h;
+	unsigned err_flags;
+
+	path->hwhcontext = hwh->context;
+	h = (struct hp_sw_context *) hwh->context;
+

you do not need the case do you?

#2 lore raw
Quoted 13 lines · Mike Christie

drivers/md/dm-hp-sw.c line 23

Quoted 8 lines · Dave Wysochanski
drivers/md/dm-hp-sw.c new file +8 -0
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_transport_fc.h>
+#include <linux/list.h>
+#include <linux/types.h>

you should do linux includes then scsi ones. Do you even need the fc, host or cmnd includes?

Indeed, some of them are unnecessary, thanks.
scsi_cmnd.h is necessary for SCSI_SENSE_BUFFERSIZE. The only other one I need is scsi/scsi.h (for START_STOP and COMMAND_SIZE). Will rearrange and fix.

Quoted 29 lines
Quoted 29 lines
drivers/md/dm-hp-sw.c new file +29 -0
+
+#include "dm.h"
+#include "dm-hw-handler.h"
+
+#define DM_MSG_PREFIX "multipath hp"
+
+struct hp_sw_context {
+	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
+};
+
+
+/**
+ * hp_sw_end_io - Completion handler for HP path activation.
+ * @req: failover request
+ * @error: scsi-ml error
+ *
+ *  Check sense data, free request structure, and notify dm that
+ *  pg initialization has completed.
+ *
+ * Context: scsi-ml softirq
+ *
+ * Possible optimizations
+ * 1. Actually check sense data for retryable error (e.g. NOT_READY)
+ */
+static void hp_sw_end_io(struct request *req, int error)
+{
+	struct dm_path *path = req->end_io_data;
+	unsigned err_flags;
+
Quoted 9 lines

drivers/md/dm-hp-sw.c line 58

Quoted 6 lines
drivers/md/dm-hp-sw.c new file +6 -0
+	if (!error) {
+		err_flags = 0;
+		DMDEBUG("hp_sw: path activation command on %s - success",
+		       	path->dev->name);
+	}
+	else {

else should be on the same line as "{"

done.

Quoted 3 lines

I also think we should handle the NOT_READY case that was reported before.

Will do.

Quoted 75 lines
Quoted 75 lines
drivers/md/dm-hp-sw.c new file +74 -0
+		DMWARN("hp_sw: path activation command on %s - error=0x%x",
+		       path->dev->name, error);
+		err_flags = MP_FAIL_PATH;
+	}
+
+	req->end_io_data = NULL;
+	__blk_put_request(req->q, req);
+	dm_pg_init_complete(path, err_flags);
+}
+
+/**
+ * hp_sw_get_request - Allocate an HP specific path activation request
+ * @path: path on which request will be sent (needed for request queue)
+ *
+ * The START command is used for path activation request.
+ * These arrays are controller-based failover, not LUN based.
+ * One START command issued to a single path will fail over all
+ * LUNs for the same controller.
+ *
+ * Possible optimizations
+ * 1. Make timeout configurable
+ * 2. Preallocate request
+ */
+static struct request *hp_sw_get_request(struct dm_path *path)
+{
+	struct request *req=NULL;
+	struct block_device *bdev = path->dev->bdev;
+	struct request_queue *q = bdev_get_queue(bdev);
+	struct hp_sw_context *h = path->hwhcontext;
+
+	req = blk_get_request(q, WRITE, GFP_ATOMIC);
+	if (req == NULL)
+		goto exit;
+
+	req->timeout = 60*HZ;
+
+	req->errors = 0;
+	req->cmd_type = REQ_TYPE_BLOCK_PC;
+	req->cmd_flags |= REQ_FAILFAST | REQ_NOMERGE;
+	req->end_io = hp_sw_end_io;
+	req->end_io_data = path;
+	req->sense = h->sense;
+	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
+
+	memset(&req->cmd, 0, BLK_MAX_CDB);
+	req->cmd[0] = START_STOP;
+	req->cmd[4] = 1;
+	req->cmd_len = COMMAND_SIZE(req->cmd[0]);
+exit:
+	return req;
+}
+
+/**
+ * hp_sw_pg_init - HP path activation implementation.
+ * @hwh: hardware handler specific data
+ * @bypassed: unused; is the path group bypassed? (see dm-mpath.c)
+ * @path: path to send initialization command
+ *
+ * Send an HP-specific path activation command on 'path'.
+ * Do not try to optimize in any way, just send the activation command.
+ * More than one path activation command may be sent to the same controller.
+ * This seems to work fine for basic failover support.
+ *
+ * Possible optimizations
+ * 1. Detect an in-progress activation request and avoid submitting another one
+ * 2. Model the controller and only send a single activation request at a time
+ * 3. Determine the state of a path before sending an activation request
+ *
+ * Context: kmpathd (see process_queued_ios() in dm-mpath.c)
+ */
+static void hp_sw_pg_init(struct hw_handler *hwh, unsigned bypassed,
+			  struct dm_path *path)
+{
+	struct request *req;
Quoted 9 lines

drivers/md/dm-hp-sw.c line 138

Quoted 6 lines
drivers/md/dm-hp-sw.c new file +6 -0
+	struct hp_sw_context *h;
+	unsigned err_flags;
+
+	path->hwhcontext = hwh->context;
+	h = (struct hp_sw_context *) hwh->context;
+

you do not need the case do you?

Do you mean "the context"? I don't understand this comment.

Also, just looked and I probably do not need err_flags in hp_sw_pg_init.