[dm-devel] [patch 2/3] Add MP_RETRY flag for hw handlers to tell dm-mpath to retry pg_init.
Dave Wysochanski
dwysocha at redhat.com
Mon Jul 30 18:54:01 UTC 2007
On Thu, 2007-07-26 at 12:15 -0700, Chandra Seetharaman wrote:
> On Thu, 2007-07-26 at 00:44 -0400, dwysocha at redhat.com wrote:
> > plain text document attachment (dm-mpath-add-retry-pg-init.patch)
> > This patch adds a MP_RETRY flag and "pg_init_retries" feature to dm-multipath
> > core. The flag is a generic one, but in this instance we use it to flag
> > cases where we must retry a pg_init command. The patch is useful for cases
> > where a hw handler sends a path initialization command to the storage and
> > it sees the command complete with an error code indicating the command
> > should be retried. In this case, the hardware handler should call
> > dm_pg_init_complete() with MP_RETRY set in the err_flags, and this suggests
> > to the dm-mpath core to retry the pg_init. However, it is not a guarantee
> > that the dm-mpath core will actually retry the pg_init. The number of actual
> > retries is governed by the multipath feature argument "pg_init_retries".
> > Once the dm-mpath core has retried the command "pg_init_retries" times
> > without success, a subsequent dm_pg_init_complete() with MP_RETRY will
> > cause the path to be failed via fail_path(). To specify a value of
> > pg_init_retries, add a line similar to the following in the 'device' section
> > of your /etc/multipath.conf file:
> > features "2 pg_init_retries 7"
> >
> >
> >
> > Index: linux-2.6.23-rc1/drivers/md/dm-hw-handler.h
> > ===================================================================
> > --- linux-2.6.23-rc1.orig/drivers/md/dm-hw-handler.h
> > +++ linux-2.6.23-rc1/drivers/md/dm-hw-handler.h
> > @@ -58,5 +58,6 @@ unsigned dm_scsi_err_handler(struct hw_h
> > #define MP_FAIL_PATH 1
> > #define MP_BYPASS_PG 2
> > #define MP_ERROR_IO 4 /* Don't retry this I/O */
> > +#define MP_RETRY 8
> >
> > #endif
> > Index: linux-2.6.23-rc1/drivers/md/dm-mpath.c
> > ===================================================================
> > --- linux-2.6.23-rc1.orig/drivers/md/dm-mpath.c
> > +++ linux-2.6.23-rc1/drivers/md/dm-mpath.c
> > @@ -75,6 +75,8 @@ struct multipath {
> > unsigned queue_io; /* Must we queue all I/O? */
> > unsigned queue_if_no_path; /* Queue I/O if last path fails? */
> > unsigned saved_queue_if_no_path;/* Saved state during suspension */
> > + unsigned pg_init_retries; /* Number of times to retry pg_init */
> > + unsigned pg_init_count; /* Number of times pg_init called */
> >
> > struct work_struct process_queued_ios;
> > struct bio_list queued_ios;
> > @@ -221,6 +223,7 @@ static void __switch_pg(struct multipath
> > if (hwh->type && hwh->type->pg_init) {
> > m->pg_init_required = 1;
> > m->queue_io = 1;
> > + m->pg_init_count = 0;
> > } else {
> > m->pg_init_required = 0;
> > m->queue_io = 0;
> > @@ -424,6 +427,7 @@ static void process_queued_ios(struct wo
> > must_queue = 0;
> >
> > if (m->pg_init_required && !m->pg_init_in_progress) {
> > + m->pg_init_count++;
> > m->pg_init_required = 0;
> > m->pg_init_in_progress = 1;
> > init_required = 1;
> > @@ -689,9 +693,11 @@ static int parse_features(struct arg_set
> > int r;
> > unsigned argc;
> > struct dm_target *ti = m->ti;
> > + char *name;
> >
> > static struct param _params[] = {
> > - {0, 1, "invalid number of feature args"},
> > + {0, 4, "invalid number of feature args"},
>
> Isn't it "3" (instead of "4") ?
>
> > + {0, 50, "invalid number of pg_init retries"},
> > };
> >
> > r = read_param(_params, shift(as), &argc, &ti->error);
> > @@ -701,12 +707,26 @@ static int parse_features(struct arg_set
> > if (!argc)
> > return 0;
> >
> > - if (!strnicmp(shift(as), MESG_STR("queue_if_no_path")))
> > - return queue_if_no_path(m, 1, 0);
> > - else {
> > - ti->error = "Unrecognised multipath feature request";
> > - return -EINVAL;
> > + while (argc && !r) {
> > + name = shift(as);
> > + argc--;
> > + if (!strnicmp(name, MESG_STR("queue_if_no_path"))) {
> > + r = queue_if_no_path(m, 1, 0);
> > + DMDEBUG("setting queue_if_no_path");
>
> Shouldn't this DEBUG be printed only when r == 0 ?
>
> > + } else if (!strnicmp(name, MESG_STR("pg_init_retries")) &&
> > + (argc >= 1)) {
>
> mixed use of space/tab.
> > + r = read_param(_params + 1, shift(as),
> > + &m->pg_init_retries, &ti->error);
> > + argc--;
> > + DMDEBUG("setting pg_init_retries to %u",
> > + m->pg_init_retries);
>
> Shouldn't this DEBUG be printed only when r == 0 ?
> > + } else {
> > + ti->error = "Unrecognised multipath feature request";
> > + return -EINVAL;
> > + }
> > }
> > +
> > + return r;
> > }
> >
> > static int multipath_ctr(struct dm_target *ti, unsigned int argc,
> > @@ -976,6 +996,21 @@ static int bypass_pg_num(struct multipat
> > }
> >
> > /*
> > + * Retry pg_init on the same path group and path
> > + */
> > +static void retry_pg(struct multipath *m, struct pgpath *pgpath)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&m->lock, flags);
> > + if (m->pg_init_count <= m->pg_init_retries)
> > + m->pg_init_required = 1;
> > + else
> > + fail_path(pgpath);
> > + spin_unlock_irqrestore(&m->lock, flags);
> > +}
> > +
> > +/*
> > * pg_init must call this when it has completed its initialisation
> > */
> > void dm_pg_init_complete(struct dm_path *path, unsigned err_flags)
> > @@ -995,8 +1030,11 @@ void dm_pg_init_complete(struct dm_path
> > if (err_flags & MP_BYPASS_PG)
> > bypass_pg(m, pg, 1);
> >
> > + if (err_flags & MP_RETRY)
> > + retry_pg(m, pgpath);
> > +
> > spin_lock_irqsave(&m->lock, flags);
> > - if (err_flags) {
> > + if (err_flags & ~MP_RETRY) {
> > m->current_pgpath = NULL;
> > m->current_pg = NULL;
> > } else if (!m->pg_init_required)
> > @@ -1149,8 +1187,13 @@ static int multipath_status(struct dm_ta
> > /* Features */
> > if (type == STATUSTYPE_INFO)
> > DMEMIT("1 %u ", m->queue_size);
> > - else if (m->queue_if_no_path)
> > + else if (m->queue_if_no_path && !m->pg_init_retries)
> > DMEMIT("1 queue_if_no_path ");
> > + else if (!m->queue_if_no_path && m->pg_init_retries)
> > + DMEMIT("2 pg_init_retries %u ", m->pg_init_retries);
> > + else if (m->queue_if_no_path && m->pg_init_retries)
> > + DMEMIT("3 queue_if_no_path pg_init_retries %u ",
> > + m->pg_init_retries);
> > else
> > DMEMIT("0 ");
> >
> >
The attached patch should address your comments. I removed the DMDEBUG
statements as they did not seem too useful beyond basic tests.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dm-mpath-add-retry-pg-init.patch
Type: text/x-patch
Size: 5468 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20070730/7276c9db/attachment.bin>
More information about the dm-devel
mailing list