All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: dm-devel@redhat.com
Cc: Mike Christie <michaelc@cs.wisc.edu>,
	Mike Anderson <andmike@linux.vnet.ibm.com>
Subject: [PATCH v2] dm mpath: revert "dm: Call blk_abort_queue on failed paths"
Date: Mon, 22 Nov 2010 20:00:25 -0500	[thread overview]
Message-ID: <20101123010025.GA18584@redhat.com> (raw)
In-Reply-To: <1290116896-18209-1-git-send-email-snitzer@redhat.com>

This reverts commit 224cb3e981f1b2f9f93dbd49eaef505d17d894c2.

Multipath was previously made to use blk_abort_queue() to allow for
lower latency path deactivation.  The call to blk_abort_queue has proven
to be unsafe due to a race (between blk_abort_queue and scsi_request_fn)
that can lead to list corruption, from:
https://www.redhat.com/archives/dm-devel/2010-November/msg00085.html

"the cmd gets blk_abort_queued/timedout run on it and the scsi eh
somehow is able to complete and run scsi_queue_insert while
scsi_request_fn is still trying to process the request."

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/md/dm-mpath.c |   12 ------------
 1 file changed, 12 deletions(-)

v2: revert commit rather than #if 0 the code in question

Index: linux-2.6/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-mpath.c
+++ linux-2.6/drivers/md/dm-mpath.c
@@ -33,7 +33,6 @@ struct pgpath {
 	unsigned fail_count;		/* Cumulative failure count */
 
 	struct dm_path path;
-	struct work_struct deactivate_path;
 	struct work_struct activate_path;
 };
 
@@ -116,7 +115,6 @@ static struct workqueue_struct *kmultipa
 static void process_queued_ios(struct work_struct *work);
 static void trigger_event(struct work_struct *work);
 static void activate_path(struct work_struct *work);
-static void deactivate_path(struct work_struct *work);
 
 
 /*-----------------------------------------------
@@ -129,7 +127,6 @@ static struct pgpath *alloc_pgpath(void)
 
 	if (pgpath) {
 		pgpath->is_active = 1;
-		INIT_WORK(&pgpath->deactivate_path, deactivate_path);
 		INIT_WORK(&pgpath->activate_path, activate_path);
 	}
 
@@ -141,14 +138,6 @@ static void free_pgpath(struct pgpath *p
 	kfree(pgpath);
 }
 
-static void deactivate_path(struct work_struct *work)
-{
-	struct pgpath *pgpath =
-		container_of(work, struct pgpath, deactivate_path);
-
-	blk_abort_queue(pgpath->path.dev->bdev->bd_disk->queue);
-}
-
 static struct priority_group *alloc_priority_group(void)
 {
 	struct priority_group *pg;
@@ -995,7 +984,6 @@ static int fail_path(struct pgpath *pgpa
 		      pgpath->path.dev->name, m->nr_valid_paths);
 
 	schedule_work(&m->trigger_event);
-	queue_work(kmultipathd, &pgpath->deactivate_path);
 
 out:
 	spin_unlock_irqrestore(&m->lock, flags);

      reply	other threads:[~2010-11-23  1:00 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-12  5:23 block_abort_queue (blk_abort_request) racing with scsi_request_fn Mike Anderson
2010-11-10  7:09 ` [dm-devel] " Mike Christie
2010-11-10  7:30   ` Mike Christie
2010-11-10 16:30     ` Mike Anderson
2010-11-10 21:16       ` Mike Christie
2010-11-12 17:54         ` Mike Anderson
2010-11-16 21:39           ` Mike Snitzer
2010-11-17 17:49             ` [dm-devel] " Mike Anderson
2010-11-17 21:55               ` Mike Snitzer
2010-11-18  4:40                 ` [PATCH v2] dm mpath: add feature flag to control call to blk_abort_queue Mike Snitzer
2010-11-18  7:20                   ` Mike Anderson
2010-11-18 15:48                     ` Mike Snitzer
2010-11-18 15:48                     ` [PATCH v3] " Mike Snitzer
2010-11-18 19:16                       ` (unknown), Mike Snitzer
2010-11-18 19:21                         ` Mike Snitzer
2010-11-18 19:19                       ` [PATCH v4] dm mpath: avoid call to blk_abort_queue by default Mike Snitzer
2010-11-18 20:07                         ` [PATCH v5] " Mike Snitzer
2010-11-18 20:18                           ` [dm-devel] " Alasdair G Kergon
2010-11-18 20:39                             ` Mike Anderson
2010-11-18 21:48                             ` [PATCH] dm mpath: disable call to blk_abort_queue and related code Mike Snitzer
2010-11-23  1:00                               ` Mike Snitzer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101123010025.GA18584@redhat.com \
    --to=snitzer@redhat.com \
    --cc=andmike@linux.vnet.ibm.com \
    --cc=dm-devel@redhat.com \
    --cc=michaelc@cs.wisc.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.