From patchwork Thu Feb 19 07:10:21 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nikanth Karthikesan X-Patchwork-Id: 7937 Received: from hormel.redhat.com (hormel1.redhat.com [209.132.177.33]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n1J7DDSV004071 for ; Thu, 19 Feb 2009 07:13:14 GMT Received: from listman.util.phx.redhat.com (listman.util.phx.redhat.com [10.8.4.110]) by hormel.redhat.com (Postfix) with ESMTP id 94F13619805; Thu, 19 Feb 2009 02:13:12 -0500 (EST) Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by listman.util.phx.redhat.com (8.13.1/8.13.1) with ESMTP id n1J7DAlX004421 for ; Thu, 19 Feb 2009 02:13:10 -0500 Received: from mx1.redhat.com (mx1.redhat.com [172.16.48.31]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n1J7D6J9027291; Thu, 19 Feb 2009 02:13:06 -0500 Received: from mx1.suse.de (ns.suse.de [195.135.220.2]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n1J7Cqnh023853; Thu, 19 Feb 2009 02:12:52 -0500 Received: from Relay2.suse.de (mail2.suse.de [195.135.221.8]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id A5A09446AE; Thu, 19 Feb 2009 08:12:51 +0100 (CET) From: Nikanth Karthikesan Organization: suse.de To: Alasdair G Kergon Date: Thu, 19 Feb 2009 12:40:21 +0530 User-Agent: KMail/1.10.3 (Linux/2.6.27.7-9-default; KDE/4.1.3; x86_64; ; ) References: <200902171917.38252.knikanth@suse.de> <20090219021118.GA3962@agk.fab.redhat.com> In-Reply-To: <20090219021118.GA3962@agk.fab.redhat.com> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200902191240.22577.knikanth@suse.de> X-RedHat-Spam-Score: -5.679 X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 X-Scanned-By: MIMEDefang 2.63 on 172.16.48.31 X-loop: dm-devel@redhat.com Cc: device-mapper development Subject: [dm-devel] Re: [PATCH] dm mpath: delay retry activate_path on SCSI_DH_RETRY X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.5 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com On Thursday 19 February 2009 07:41:18 Alasdair G Kergon wrote: > On Tue, Feb 17, 2009 at 07:17:37PM +0530, Nikanth Karthikesan wrote: > > Delay retry to activate_path if it returns SCSI_DH_RETRY. > > Please write a complete patch header if you'd like this queued for > upstream! > > E.g. Why? Any cases where this will make matters worse or make no > difference? > Sorry for the terse changelog. > > + unsigned long pg_init_jiffy; /* To delay retry if SCSI_DH_RETRY */ > > +#define SCSI_DH_RETRY_DELAY ((HZ * 2)) > > Why that particular value? There is no specific reason. Just picked 2 seconds based on the TODO comment, "For SCSI_DH_RETRY we should wait a couple seconds". > Please move it into a header file with an explanation of what it does. > done. Attached is the patch with comments from you and Chandra incorporated. Thanks Nikanth SCSI Device Handlers return SCSI_DH_IMM_RETRY if we could retry immediately and SCSI_DH_RETRY in cases where it is better to retry after some delay. Currently we retry immediately irrespective of SCSI_DH_IMM_RETRY and SCSI_DH_RETRY. This patch adds a 2 second delay before retrying to activate a device, if it returns SCSI_DH_RETRY. Signed-off-by: Nikanth Karthikesan --- -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 095f77b..6fd76f1 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -65,12 +65,13 @@ struct multipath { spinlock_t lock; const char *hw_handler_name; - struct work_struct activate_path; + struct delayed_work activate_path; struct pgpath *pgpath_to_activate; unsigned nr_priority_groups; struct list_head priority_groups; unsigned pg_init_required; /* pg_init needs calling? */ unsigned pg_init_in_progress; /* Only one pg_init allowed at once */ + unsigned pg_init_delay; /* delay required before retry? */ unsigned nr_valid_paths; /* Total number of usable paths */ struct pgpath *current_pgpath; @@ -203,7 +204,7 @@ static struct multipath *alloc_multipath(struct dm_target *ti) m->queue_io = 1; INIT_WORK(&m->process_queued_ios, process_queued_ios); INIT_WORK(&m->trigger_event, trigger_event); - INIT_WORK(&m->activate_path, activate_path); + INIT_DELAYED_WORK(&m->activate_path, activate_path); m->mpio_pool = mempool_create_slab_pool(MIN_IOS, _mpio_cache); if (!m->mpio_pool) { kfree(m); @@ -431,6 +432,7 @@ static void process_queued_ios(struct work_struct *work) struct pgpath *pgpath = NULL; unsigned init_required = 0, must_queue = 1; unsigned long flags; + unsigned long delay = 0; spin_lock_irqsave(&m->lock, flags); @@ -452,13 +454,15 @@ static void process_queued_ios(struct work_struct *work) m->pg_init_required = 0; m->pg_init_in_progress = 1; init_required = 1; + if (m->pg_init_delay) + delay = SCSI_DH_RETRY_DELAY; } out: spin_unlock_irqrestore(&m->lock, flags); if (init_required) - queue_work(kmpath_handlerd, &m->activate_path); + queue_delayed_work(kmpath_handlerd, &m->activate_path, delay); if (!must_queue) dispatch_queued_ios(m); @@ -1060,6 +1064,7 @@ static void pg_init_done(struct dm_path *path, int errors) struct priority_group *pg = pgpath->pg; struct multipath *m = pg->m; unsigned long flags; + unsigned delay = 0; /* device or driver problems */ switch (errors) { @@ -1084,8 +1089,11 @@ static void pg_init_done(struct dm_path *path, int errors) */ bypass_pg(m, pg, 1); break; - /* TODO: For SCSI_DH_RETRY we should wait a couple seconds */ + /* + * For SCSI_DH_RETRY we wait before retrying. + */ case SCSI_DH_RETRY: + delay = 1; case SCSI_DH_IMM_RETRY: case SCSI_DH_RES_TEMP_UNAVAIL: if (pg_init_limit_reached(m, pgpath)) @@ -1112,6 +1120,7 @@ static void pg_init_done(struct dm_path *path, int errors) } m->pg_init_in_progress = 0; + m->pg_init_delay = delay; queue_work(kmultipathd, &m->process_queued_ios); spin_unlock_irqrestore(&m->lock, flags); } @@ -1120,7 +1129,7 @@ static void activate_path(struct work_struct *work) { int ret; struct multipath *m = - container_of(work, struct multipath, activate_path); + container_of(work, struct multipath, activate_path.work); struct dm_path *path; unsigned long flags; diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h index 33efce2..f099d86 100644 --- a/include/scsi/scsi_dh.h +++ b/include/scsi/scsi_dh.h @@ -55,6 +55,10 @@ enum { SCSI_DH_NOSYS, SCSI_DH_DRIVER_MAX, }; + +/* Time to wait before retry in case of SCSI_DH_RETRY */ +#define SCSI_DH_RETRY_DELAY ((HZ * 2)) + #if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE) extern int scsi_dh_activate(struct request_queue *); extern int scsi_dh_handler_exist(const char *);