* [PATCHv9 0/8] dm-multipath: push back requests instead of queueing
@ 2014-02-28 14:33 Hannes Reinecke
2014-02-28 14:33 ` [PATCH 1/8] dm mpath: do not call pg_init when it is already running Hannes Reinecke
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
Hi Mike,
here's version 9 of the patchset which might just be the
final one. I've included the remaining issue Jun'ichi pointed
out, and added the respective 'reviewed-by' tags where missing.
Hannes Reinecke (6):
dm mpath: do not call pg_init when it is already running
dm mpath: push back requests instead of queueing
dm mpath: remove process_queued_ios()
dm mpath: reduce memory pressure when requeuing
dm mpath: remove map_io()
dm-mpath: do not activate failed paths
Mike Snitzer (2):
dm table: add dm_table_run_md_queue_async
dm mpath: remove extra nesting in map function
drivers/md/dm-mpath.c | 213 +++++++++++++++---------------------------
drivers/md/dm-table.c | 19 ++++
drivers/md/dm.c | 5 +
drivers/md/dm.h | 1 +
include/linux/device-mapper.h | 5 +
5 files changed, 105 insertions(+), 138 deletions(-)
--
1.7.12.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/8] dm mpath: do not call pg_init when it is already running
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 2/8] dm table: add dm_table_run_md_queue_async Hannes Reinecke
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
This patch moves condition checks as a preparation of following
patches and has no effect on behaviour.
process_queued_ios() is the only caller of __pg_init_all_paths()
and 2 condition checks are moved from outside to inside without
side effects.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index e8431fb..bb2f6b2 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -261,6 +261,9 @@ static void __pg_init_all_paths(struct multipath *m)
struct pgpath *pgpath;
unsigned long pg_init_delay = 0;
+ if (m->pg_init_in_progress || m->pg_init_disabled)
+ return;
+
m->pg_init_count++;
m->pg_init_required = 0;
if (m->pg_init_delay_retry)
@@ -501,8 +504,7 @@ static void process_queued_ios(struct work_struct *work)
(!pgpath && !m->queue_if_no_path))
must_queue = 0;
- if (m->pg_init_required && !m->pg_init_in_progress && pgpath &&
- !m->pg_init_disabled)
+ if (pgpath && m->pg_init_required)
__pg_init_all_paths(m);
spin_unlock_irqrestore(&m->lock, flags);
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/8] dm table: add dm_table_run_md_queue_async
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
2014-02-28 14:33 ` [PATCH 1/8] dm mpath: do not call pg_init when it is already running Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 3/8] dm mpath: push back requests instead of queueing Hannes Reinecke
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
From: Mike Snitzer <snitzer@redhat.com>
Introduce dm_table_run_md_queue_async() to run the request_queue of the
mapped_device associated with a request-based DM table.
Also add dm_md_get_queue() wrapper to extract the request_queue from a
mapped_device.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-table.c | 19 +++++++++++++++++++
drivers/md/dm.c | 5 +++++
drivers/md/dm.h | 1 +
include/linux/device-mapper.h | 5 +++++
4 files changed, 30 insertions(+)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 6a7f2b8..8df63ed 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1618,6 +1618,25 @@ struct mapped_device *dm_table_get_md(struct dm_table *t)
}
EXPORT_SYMBOL(dm_table_get_md);
+void dm_table_run_md_queue_async(struct dm_table *t)
+{
+ struct mapped_device *md;
+ struct request_queue *queue;
+ unsigned long flags;
+
+ if (!dm_table_request_based(t))
+ return;
+
+ md = dm_table_get_md(t);
+ queue = dm_get_md_queue(md);
+ if (queue) {
+ spin_lock_irqsave(queue->queue_lock, flags);
+ blk_run_queue_async(queue);
+ spin_unlock_irqrestore(queue->queue_lock, flags);
+ }
+}
+EXPORT_SYMBOL(dm_table_run_md_queue_async);
+
static int device_discard_capable(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8c53b09..341991f 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -475,6 +475,11 @@ sector_t dm_get_size(struct mapped_device *md)
return get_capacity(md->disk);
}
+struct request_queue *dm_get_md_queue(struct mapped_device *md)
+{
+ return md->queue;
+}
+
struct dm_stats *dm_get_stats(struct mapped_device *md)
{
return &md->stats;
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index c4569f0..1b2ca8a 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -189,6 +189,7 @@ int dm_lock_for_deletion(struct mapped_device *md, bool mark_deferred, bool only
int dm_cancel_deferred_remove(struct mapped_device *md);
int dm_request_based(struct mapped_device *md);
sector_t dm_get_size(struct mapped_device *md);
+struct request_queue *dm_get_md_queue(struct mapped_device *md);
struct dm_stats *dm_get_stats(struct mapped_device *md);
int dm_kobject_uevent(struct mapped_device *md, enum kobject_action action,
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index ed419c6..250225b 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -466,6 +466,11 @@ struct mapped_device *dm_table_get_md(struct dm_table *t);
void dm_table_event(struct dm_table *t);
/*
+ * Run the queue for request-based targets.
+ */
+void dm_table_run_md_queue_async(struct dm_table *t);
+
+/*
* The device must be suspended before calling this method.
* Returns the previous table, which the caller must destroy.
*/
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/8] dm mpath: push back requests instead of queueing
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
2014-02-28 14:33 ` [PATCH 1/8] dm mpath: do not call pg_init when it is already running Hannes Reinecke
2014-02-28 14:33 ` [PATCH 2/8] dm table: add dm_table_run_md_queue_async Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
There is no reason why multipath needs to queue requests internally for
queue_if_no_path or pg_init; we should rather push them back onto the
request queue.
And while we're at it we can simplify the conditional statement in
map_io() to make it easier to read.
Since mpath no longer does internal queuing of I/O the table info no
longer emits the internal queue_size. Instead it displays 1 if queuing
is being used or 0 if it is not.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 114 ++++++++++++++++----------------------------------
1 file changed, 36 insertions(+), 78 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index bb2f6b2..1c6ca5c 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -93,9 +93,7 @@ struct multipath {
unsigned pg_init_count; /* Number of times pg_init called */
unsigned pg_init_delay_msecs; /* Number of msecs before pg_init retry */
- unsigned queue_size;
struct work_struct process_queued_ios;
- struct list_head queued_ios;
struct work_struct trigger_event;
@@ -124,6 +122,7 @@ static struct workqueue_struct *kmultipathd, *kmpath_handlerd;
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 int __pgpath_busy(struct pgpath *pgpath);
/*-----------------------------------------------
@@ -195,7 +194,6 @@ static struct multipath *alloc_multipath(struct dm_target *ti)
m = kzalloc(sizeof(*m), GFP_KERNEL);
if (m) {
INIT_LIST_HEAD(&m->priority_groups);
- INIT_LIST_HEAD(&m->queued_ios);
spin_lock_init(&m->lock);
m->queue_io = 1;
m->pg_init_delay_msecs = DM_PG_INIT_DELAY_DEFAULT;
@@ -368,12 +366,15 @@ failed:
*/
static int __must_push_back(struct multipath *m)
{
- return (m->queue_if_no_path != m->saved_queue_if_no_path &&
- dm_noflush_suspending(m->ti));
+ return (m->queue_if_no_path ||
+ (m->queue_if_no_path != m->saved_queue_if_no_path &&
+ dm_noflush_suspending(m->ti)));
}
+#define pg_ready(m) (!(m)->queue_io && !(m)->pg_init_required)
+
static int map_io(struct multipath *m, struct request *clone,
- union map_info *map_context, unsigned was_queued)
+ union map_info *map_context)
{
int r = DM_MAPIO_REMAPPED;
size_t nr_bytes = blk_rq_bytes(clone);
@@ -391,37 +392,28 @@ static int map_io(struct multipath *m, struct request *clone,
pgpath = m->current_pgpath;
- if (was_queued)
- m->queue_size--;
-
- if (m->pg_init_required) {
- if (!m->pg_init_in_progress)
- queue_work(kmultipathd, &m->process_queued_ios);
- r = DM_MAPIO_REQUEUE;
- } else if ((pgpath && m->queue_io) ||
- (!pgpath && m->queue_if_no_path)) {
- /* Queue for the daemon to resubmit */
- list_add_tail(&clone->queuelist, &m->queued_ios);
- m->queue_size++;
- if (!m->queue_io)
- queue_work(kmultipathd, &m->process_queued_ios);
- pgpath = NULL;
- r = DM_MAPIO_SUBMITTED;
- } else if (pgpath) {
- bdev = pgpath->path.dev->bdev;
- clone->q = bdev_get_queue(bdev);
- clone->rq_disk = bdev->bd_disk;
- } else if (__must_push_back(m))
- r = DM_MAPIO_REQUEUE;
- else
- r = -EIO; /* Failed */
-
- mpio->pgpath = pgpath;
- mpio->nr_bytes = nr_bytes;
-
- if (r == DM_MAPIO_REMAPPED && pgpath->pg->ps.type->start_io)
- pgpath->pg->ps.type->start_io(&pgpath->pg->ps, &pgpath->path,
- nr_bytes);
+ if (pgpath) {
+ if (pg_ready(m)) {
+ bdev = pgpath->path.dev->bdev;
+ clone->q = bdev_get_queue(bdev);
+ clone->rq_disk = bdev->bd_disk;
+ mpio->pgpath = pgpath;
+ mpio->nr_bytes = nr_bytes;
+ if (pgpath->pg->ps.type->start_io)
+ pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
+ &pgpath->path,
+ nr_bytes);
+ } else {
+ __pg_init_all_paths(m);
+ r = DM_MAPIO_REQUEUE;
+ }
+ } else {
+ /* No path */
+ if (__must_push_back(m))
+ r = DM_MAPIO_REQUEUE;
+ else
+ r = -EIO; /* Failed */
+ }
spin_unlock_irqrestore(&m->lock, flags);
@@ -443,7 +435,7 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path,
else
m->saved_queue_if_no_path = queue_if_no_path;
m->queue_if_no_path = queue_if_no_path;
- if (!m->queue_if_no_path && m->queue_size)
+ if (!m->queue_if_no_path)
queue_work(kmultipathd, &m->process_queued_ios);
spin_unlock_irqrestore(&m->lock, flags);
@@ -451,40 +443,6 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path,
return 0;
}
-/*-----------------------------------------------------------------
- * The multipath daemon is responsible for resubmitting queued ios.
- *---------------------------------------------------------------*/
-
-static void dispatch_queued_ios(struct multipath *m)
-{
- int r;
- unsigned long flags;
- union map_info *info;
- struct request *clone, *n;
- LIST_HEAD(cl);
-
- spin_lock_irqsave(&m->lock, flags);
- list_splice_init(&m->queued_ios, &cl);
- spin_unlock_irqrestore(&m->lock, flags);
-
- list_for_each_entry_safe(clone, n, &cl, queuelist) {
- list_del_init(&clone->queuelist);
-
- info = dm_get_rq_mapinfo(clone);
-
- r = map_io(m, clone, info, 1);
- if (r < 0) {
- clear_mapinfo(m, info);
- dm_kill_unmapped_request(clone, r);
- } else if (r == DM_MAPIO_REMAPPED)
- dm_dispatch_request(clone);
- else if (r == DM_MAPIO_REQUEUE) {
- clear_mapinfo(m, info);
- dm_requeue_unmapped_request(clone);
- }
- }
-}
-
static void process_queued_ios(struct work_struct *work)
{
struct multipath *m =
@@ -509,7 +467,7 @@ static void process_queued_ios(struct work_struct *work)
spin_unlock_irqrestore(&m->lock, flags);
if (!must_queue)
- dispatch_queued_ios(m);
+ dm_table_run_md_queue_async(m->ti->table);
}
/*
@@ -987,7 +945,7 @@ static int multipath_map(struct dm_target *ti, struct request *clone,
return DM_MAPIO_REQUEUE;
clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
- r = map_io(m, clone, map_context, 0);
+ r = map_io(m, clone, map_context);
if (r < 0 || r == DM_MAPIO_REQUEUE)
clear_mapinfo(m, map_context);
@@ -1056,7 +1014,7 @@ static int reinstate_path(struct pgpath *pgpath)
pgpath->is_active = 1;
- if (!m->nr_valid_paths++ && m->queue_size) {
+ if (!m->nr_valid_paths++) {
m->current_pgpath = NULL;
queue_work(kmultipathd, &m->process_queued_ios);
} else if (m->hw_handler_name && (m->current_pg == pgpath->pg)) {
@@ -1435,7 +1393,7 @@ static void multipath_status(struct dm_target *ti, status_type_t type,
/* Features */
if (type == STATUSTYPE_INFO)
- DMEMIT("2 %u %u ", m->queue_size, m->pg_init_count);
+ DMEMIT("2 %u %u ", m->queue_io, m->pg_init_count);
else {
DMEMIT("%u ", m->queue_if_no_path +
(m->pg_init_retries > 0) * 2 +
@@ -1688,7 +1646,7 @@ static int multipath_busy(struct dm_target *ti)
spin_lock_irqsave(&m->lock, flags);
/* pg_init in progress, requeue until done */
- if (m->pg_init_in_progress) {
+ if (!pg_ready(m)) {
busy = 1;
goto out;
}
@@ -1741,7 +1699,7 @@ out:
*---------------------------------------------------------------*/
static struct target_type multipath_target = {
.name = "multipath",
- .version = {1, 6, 0},
+ .version = {1, 7, 0},
.module = THIS_MODULE,
.ctr = multipath_ctr,
.dtr = multipath_dtr,
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (2 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 3/8] dm mpath: push back requests instead of queueing Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-03-02 23:12 ` Junichi Nomura
2014-02-28 14:33 ` [PATCH 5/8] dm mpath: reduce memory pressure when requeuing Hannes Reinecke
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
process_queued_ios() has served 3 functions:
1) select pg and pgpath if none is selected
2) start pg_init if requested
3) dispatch queued IOs when pg is ready
Basically, a call to queue_work(process_queued_ios) can be replaced by
dm_table_run_md_queue_async(), which runs request queue and ends up
calling map_io(), which does 1), 2) and 3).
Exception is when !pg_ready() (which means either pg_init is running or
requested), then multipath_busy() prevents map_io() being called from
request_fn.
If pg_init is running, it should be ok as long as pg_init_done() does
the right thing when pg_init is completed, I.e.: restart pg_init if
!pg_ready() or call dm_table_run_md_queue_async() to kick map_io().
If pg_init is requested, we have to make sure the request is detected
and pg_init will be started. pg_init is requested in 3 places:
a) __choose_pgpath() in map_io()
b) __choose_pgpath() in multipath_ioctl()
c) pg_init retry in pg_init_done()
a) is ok because map_io() calls __pg_init_all_paths(), which does 2).
b) needs a call to __pg_init_all_paths(), which does 2).
c) needs a call to __pg_init_all_paths(), which does 2).
So this patch removes process_queued_ios() and ensures that
__pg_init_all_paths() is called at the appropriate locations.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 67 +++++++++++++++++++++------------------------------
1 file changed, 27 insertions(+), 40 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 1c6ca5c..a26980b 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -93,8 +93,6 @@ struct multipath {
unsigned pg_init_count; /* Number of times pg_init called */
unsigned pg_init_delay_msecs; /* Number of msecs before pg_init retry */
- struct work_struct process_queued_ios;
-
struct work_struct trigger_event;
/*
@@ -119,7 +117,6 @@ typedef int (*action_fn) (struct pgpath *pgpath);
static struct kmem_cache *_mpio_cache;
static struct workqueue_struct *kmultipathd, *kmpath_handlerd;
-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 int __pgpath_busy(struct pgpath *pgpath);
@@ -197,7 +194,6 @@ static struct multipath *alloc_multipath(struct dm_target *ti)
spin_lock_init(&m->lock);
m->queue_io = 1;
m->pg_init_delay_msecs = DM_PG_INIT_DELAY_DEFAULT;
- INIT_WORK(&m->process_queued_ios, process_queued_ios);
INIT_WORK(&m->trigger_event, trigger_event);
init_waitqueue_head(&m->pg_init_wait);
mutex_init(&m->work_mutex);
@@ -254,16 +250,21 @@ static void clear_mapinfo(struct multipath *m, union map_info *info)
* Path selection
*-----------------------------------------------*/
-static void __pg_init_all_paths(struct multipath *m)
+static int __pg_init_all_paths(struct multipath *m)
{
struct pgpath *pgpath;
unsigned long pg_init_delay = 0;
if (m->pg_init_in_progress || m->pg_init_disabled)
- return;
+ return 0;
m->pg_init_count++;
m->pg_init_required = 0;
+
+ /* Check here to reset pg_init_required */
+ if (!m->current_pg)
+ return 0;
+
if (m->pg_init_delay_retry)
pg_init_delay = msecs_to_jiffies(m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT ?
m->pg_init_delay_msecs : DM_PG_INIT_DELAY_MSECS);
@@ -275,6 +276,7 @@ static void __pg_init_all_paths(struct multipath *m)
pg_init_delay))
m->pg_init_in_progress++;
}
+ return m->pg_init_in_progress;
}
static void __switch_pg(struct multipath *m, struct pgpath *pgpath)
@@ -436,40 +438,13 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path,
m->saved_queue_if_no_path = queue_if_no_path;
m->queue_if_no_path = queue_if_no_path;
if (!m->queue_if_no_path)
- queue_work(kmultipathd, &m->process_queued_ios);
+ dm_table_run_md_queue_async(m->ti->table);
spin_unlock_irqrestore(&m->lock, flags);
return 0;
}
-static void process_queued_ios(struct work_struct *work)
-{
- struct multipath *m =
- container_of(work, struct multipath, process_queued_ios);
- struct pgpath *pgpath = NULL;
- unsigned must_queue = 1;
- unsigned long flags;
-
- spin_lock_irqsave(&m->lock, flags);
-
- if (!m->current_pgpath)
- __choose_pgpath(m, 0);
-
- pgpath = m->current_pgpath;
-
- if ((pgpath && !m->queue_io) ||
- (!pgpath && !m->queue_if_no_path))
- must_queue = 0;
-
- if (pgpath && m->pg_init_required)
- __pg_init_all_paths(m);
-
- spin_unlock_irqrestore(&m->lock, flags);
- if (!must_queue)
- dm_table_run_md_queue_async(m->ti->table);
-}
-
/*
* An event is triggered whenever a path is taken out of use.
* Includes path failure and PG bypass.
@@ -1016,7 +991,7 @@ static int reinstate_path(struct pgpath *pgpath)
if (!m->nr_valid_paths++) {
m->current_pgpath = NULL;
- queue_work(kmultipathd, &m->process_queued_ios);
+ dm_table_run_md_queue_async(m->ti->table);
} else if (m->hw_handler_name && (m->current_pg == pgpath->pg)) {
if (queue_work(kmpath_handlerd, &pgpath->activate_path.work))
m->pg_init_in_progress++;
@@ -1214,9 +1189,12 @@ static void pg_init_done(void *data, int errors)
if (!m->pg_init_required)
m->queue_io = 0;
-
- m->pg_init_delay_retry = delay_retry;
- queue_work(kmultipathd, &m->process_queued_ios);
+ else if (m->current_pg) {
+ m->pg_init_delay_retry = delay_retry;
+ if (__pg_init_all_paths(m))
+ goto out;
+ m->queue_io = 0;
+ }
/*
* Wake up any thread waiting to suspend.
@@ -1594,8 +1572,17 @@ static int multipath_ioctl(struct dm_target *ti, unsigned int cmd,
r = err;
}
- if (r == -ENOTCONN && !fatal_signal_pending(current))
- queue_work(kmultipathd, &m->process_queued_ios);
+ if (r == -ENOTCONN && !fatal_signal_pending(current)) {
+ spin_lock_irqsave(&m->lock, flags);
+ if (!m->current_pg) {
+ /* Path status changed, redo selection */
+ __choose_pgpath(m, 0);
+ }
+ if (m->pg_init_required)
+ __pg_init_all_paths(m);
+ spin_unlock_irqrestore(&m->lock, flags);
+ dm_table_run_md_queue_async(m->ti->table);
+ }
return r ? : __blkdev_driver_ioctl(bdev, mode, cmd, arg);
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/8] dm mpath: reduce memory pressure when requeuing
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (3 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 6/8] dm mpath: remove map_io() Hannes Reinecke
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
When multipath needs to requeue I/O in the block layer the per-request
context shouldn't be allocated, as it will be freed immediately
afterwards anyway. Avoiding this memory allocation will reduce memory
pressure during requeuing.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index a26980b..13a855b 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -378,12 +378,12 @@ static int __must_push_back(struct multipath *m)
static int map_io(struct multipath *m, struct request *clone,
union map_info *map_context)
{
- int r = DM_MAPIO_REMAPPED;
+ int r = DM_MAPIO_REQUEUE;
size_t nr_bytes = blk_rq_bytes(clone);
unsigned long flags;
struct pgpath *pgpath;
struct block_device *bdev;
- struct dm_mpath_io *mpio = map_context->ptr;
+ struct dm_mpath_io *mpio;
spin_lock_irqsave(&m->lock, flags);
@@ -396,27 +396,29 @@ static int map_io(struct multipath *m, struct request *clone,
if (pgpath) {
if (pg_ready(m)) {
+ if (set_mapinfo(m, map_context) < 0)
+ /* ENOMEM, requeue */
+ goto out_unlock;
+
bdev = pgpath->path.dev->bdev;
clone->q = bdev_get_queue(bdev);
clone->rq_disk = bdev->bd_disk;
+ clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
+ mpio = map_context->ptr;
mpio->pgpath = pgpath;
mpio->nr_bytes = nr_bytes;
if (pgpath->pg->ps.type->start_io)
pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
&pgpath->path,
nr_bytes);
- } else {
- __pg_init_all_paths(m);
- r = DM_MAPIO_REQUEUE;
+ r = DM_MAPIO_REMAPPED;
+ goto out_unlock;
}
- } else {
- /* No path */
- if (__must_push_back(m))
- r = DM_MAPIO_REQUEUE;
- else
- r = -EIO; /* Failed */
- }
+ __pg_init_all_paths(m);
+ } else if (!__must_push_back(m))
+ r = -EIO; /* Failed */
+out_unlock:
spin_unlock_irqrestore(&m->lock, flags);
return r;
@@ -912,19 +914,9 @@ static void multipath_dtr(struct dm_target *ti)
static int multipath_map(struct dm_target *ti, struct request *clone,
union map_info *map_context)
{
- int r;
struct multipath *m = (struct multipath *) ti->private;
- if (set_mapinfo(m, map_context) < 0)
- /* ENOMEM, requeue */
- return DM_MAPIO_REQUEUE;
-
- clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
- r = map_io(m, clone, map_context);
- if (r < 0 || r == DM_MAPIO_REQUEUE)
- clear_mapinfo(m, map_context);
-
- return r;
+ return map_io(m, clone, map_context);
}
/*
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/8] dm mpath: remove map_io()
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (4 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 5/8] dm mpath: reduce memory pressure when requeuing Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 7/8] dm mpath: remove extra nesting in map function Hannes Reinecke
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
multipath_map() is now just a wrapper around map_io(), so we
can rename map_io() to multipath_map().
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 13a855b..5e92d51 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -375,9 +375,13 @@ static int __must_push_back(struct multipath *m)
#define pg_ready(m) (!(m)->queue_io && !(m)->pg_init_required)
-static int map_io(struct multipath *m, struct request *clone,
- union map_info *map_context)
+/*
+ * Map cloned requests
+ */
+static int multipath_map(struct dm_target *ti, struct request *clone,
+ union map_info *map_context)
{
+ struct multipath *m = (struct multipath *) ti->private;
int r = DM_MAPIO_REQUEUE;
size_t nr_bytes = blk_rq_bytes(clone);
unsigned long flags;
@@ -909,17 +913,6 @@ static void multipath_dtr(struct dm_target *ti)
}
/*
- * Map cloned requests
- */
-static int multipath_map(struct dm_target *ti, struct request *clone,
- union map_info *map_context)
-{
- struct multipath *m = (struct multipath *) ti->private;
-
- return map_io(m, clone, map_context);
-}
-
-/*
* Take a path out of use.
*/
static int fail_path(struct pgpath *pgpath)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/8] dm mpath: remove extra nesting in map function
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (5 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 6/8] dm mpath: remove map_io() Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-02-28 14:33 ` [PATCH 8/8] dm-mpath: do not activate failed paths Hannes Reinecke
2014-03-12 21:46 ` [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Mike Snitzer
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
From: Mike Snitzer <snitzer@redhat.com>
Return early for case when no path exists, and when the
pathgroup isn't ready. This eliminates the need for
extra nesting for the the common case.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/md/dm-mpath.c | 46 ++++++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 5e92d51..eaf4855 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -398,29 +398,31 @@ static int multipath_map(struct dm_target *ti, struct request *clone,
pgpath = m->current_pgpath;
- if (pgpath) {
- if (pg_ready(m)) {
- if (set_mapinfo(m, map_context) < 0)
- /* ENOMEM, requeue */
- goto out_unlock;
-
- bdev = pgpath->path.dev->bdev;
- clone->q = bdev_get_queue(bdev);
- clone->rq_disk = bdev->bd_disk;
- clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
- mpio = map_context->ptr;
- mpio->pgpath = pgpath;
- mpio->nr_bytes = nr_bytes;
- if (pgpath->pg->ps.type->start_io)
- pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
- &pgpath->path,
- nr_bytes);
- r = DM_MAPIO_REMAPPED;
- goto out_unlock;
- }
+ if (!pgpath) {
+ if (!__must_push_back(m))
+ r = -EIO; /* Failed */
+ goto out_unlock;
+ }
+ if (!pg_ready(m)) {
__pg_init_all_paths(m);
- } else if (!__must_push_back(m))
- r = -EIO; /* Failed */
+ goto out_unlock;
+ }
+ if (set_mapinfo(m, map_context) < 0)
+ /* ENOMEM, requeue */
+ goto out_unlock;
+
+ bdev = pgpath->path.dev->bdev;
+ clone->q = bdev_get_queue(bdev);
+ clone->rq_disk = bdev->bd_disk;
+ clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
+ mpio = map_context->ptr;
+ mpio->pgpath = pgpath;
+ mpio->nr_bytes = nr_bytes;
+ if (pgpath->pg->ps.type->start_io)
+ pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
+ &pgpath->path,
+ nr_bytes);
+ r = DM_MAPIO_REMAPPED;
out_unlock:
spin_unlock_irqrestore(&m->lock, flags);
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 8/8] dm-mpath: do not activate failed paths
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (6 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 7/8] dm mpath: remove extra nesting in map function Hannes Reinecke
@ 2014-02-28 14:33 ` Hannes Reinecke
2014-03-12 21:46 ` [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Mike Snitzer
8 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 14:33 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
activate_path() is run without a lock, so the path might be
set to failed before activate_path() had a chance to run.
This patch add a check for ->active in activate_path() to
avoid unnecessary overhead by calling functions which are known
to be failing.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
---
drivers/md/dm-mpath.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index eaf4855..e750095 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1197,8 +1197,11 @@ static void activate_path(struct work_struct *work)
struct pgpath *pgpath =
container_of(work, struct pgpath, activate_path.work);
- scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev),
- pg_init_done, pgpath);
+ if (pgpath->is_active)
+ scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev),
+ pg_init_done, pgpath);
+ else
+ pg_init_done(pgpath, SCSI_DH_DEV_OFFLINED);
}
static int noretry_error(int error)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-02-28 14:33 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
@ 2014-03-02 23:12 ` Junichi Nomura
2014-03-05 8:59 ` Hannes Reinecke
0 siblings, 1 reply; 17+ messages in thread
From: Junichi Nomura @ 2014-03-02 23:12 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: dm-devel, Mike Snitzer, Alasdair Kergon
On 02/28/14 23:33, Hannes Reinecke wrote:
> @@ -1214,9 +1189,12 @@ static void pg_init_done(void *data, int errors)
>
> if (!m->pg_init_required)
> m->queue_io = 0;
> -
> - m->pg_init_delay_retry = delay_retry;
> - queue_work(kmultipathd, &m->process_queued_ios);
> + else if (m->current_pg) {
> + m->pg_init_delay_retry = delay_retry;
> + if (__pg_init_all_paths(m))
> + goto out;
> + m->queue_io = 0;
> + }
Hi Hannes,
If m->pg_init_required && !m->current_pg,
doesn't m->queue_io remain 1 while nobody kicks pg_init?
That's why I suggested this:
if (m->pg_init_required) {
m->pg_init_delay_retry = delay_retry;
if (__pg_init_all_paths(m))
goto out;
}
m->queue_io = 0;
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-03-02 23:12 ` Junichi Nomura
@ 2014-03-05 8:59 ` Hannes Reinecke
2014-03-05 14:24 ` Mike Snitzer
0 siblings, 1 reply; 17+ messages in thread
From: Hannes Reinecke @ 2014-03-05 8:59 UTC (permalink / raw)
To: Junichi Nomura; +Cc: dm-devel, Mike Snitzer, Alasdair Kergon
On 03/03/2014 12:12 AM, Junichi Nomura wrote:
> On 02/28/14 23:33, Hannes Reinecke wrote:
>> @@ -1214,9 +1189,12 @@ static void pg_init_done(void *data, int errors)
>>
>> if (!m->pg_init_required)
>> m->queue_io = 0;
>> -
>> - m->pg_init_delay_retry = delay_retry;
>> - queue_work(kmultipathd, &m->process_queued_ios);
>> + else if (m->current_pg) {
>> + m->pg_init_delay_retry = delay_retry;
>> + if (__pg_init_all_paths(m))
>> + goto out;
>> + m->queue_io = 0;
>> + }
>
> Hi Hannes,
>
> If m->pg_init_required && !m->current_pg,
> doesn't m->queue_io remain 1 while nobody kicks pg_init?
>
> That's why I suggested this:
>
> if (m->pg_init_required) {
> m->pg_init_delay_retry = delay_retry;
> if (__pg_init_all_paths(m))
> goto out;
> }
> m->queue_io = 0;
>
Yeah, sorry. You are right.
Mike, can you fix it up or do you need an additional round of patches?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-03-05 8:59 ` Hannes Reinecke
@ 2014-03-05 14:24 ` Mike Snitzer
0 siblings, 0 replies; 17+ messages in thread
From: Mike Snitzer @ 2014-03-05 14:24 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Junichi Nomura, dm-devel, Alasdair Kergon
On Wed, Mar 05 2014 at 3:59am -0500,
Hannes Reinecke <hare@suse.de> wrote:
> On 03/03/2014 12:12 AM, Junichi Nomura wrote:
> > On 02/28/14 23:33, Hannes Reinecke wrote:
> >> @@ -1214,9 +1189,12 @@ static void pg_init_done(void *data, int errors)
> >>
> >> if (!m->pg_init_required)
> >> m->queue_io = 0;
> >> -
> >> - m->pg_init_delay_retry = delay_retry;
> >> - queue_work(kmultipathd, &m->process_queued_ios);
> >> + else if (m->current_pg) {
> >> + m->pg_init_delay_retry = delay_retry;
> >> + if (__pg_init_all_paths(m))
> >> + goto out;
> >> + m->queue_io = 0;
> >> + }
> >
> > Hi Hannes,
> >
> > If m->pg_init_required && !m->current_pg,
> > doesn't m->queue_io remain 1 while nobody kicks pg_init?
> >
> > That's why I suggested this:
> >
> > if (m->pg_init_required) {
> > m->pg_init_delay_retry = delay_retry;
> > if (__pg_init_all_paths(m))
> > goto out;
> > }
> > m->queue_io = 0;
> >
> Yeah, sorry. You are right.
>
> Mike, can you fix it up or do you need an additional round of patches?
I'll take care of it. I won't be looking at these patches until next
week though.
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv9 0/8] dm-multipath: push back requests instead of queueing
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
` (7 preceding siblings ...)
2014-02-28 14:33 ` [PATCH 8/8] dm-mpath: do not activate failed paths Hannes Reinecke
@ 2014-03-12 21:46 ` Mike Snitzer
2014-03-13 6:46 ` Hannes Reinecke
8 siblings, 1 reply; 17+ messages in thread
From: Mike Snitzer @ 2014-03-12 21:46 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Jun'ichi Nomura, dm-devel, Alasdair Kergon
On Fri, Feb 28 2014 at 9:33am -0500,
Hannes Reinecke <hare@suse.de> wrote:
> Hi Mike,
>
> here's version 9 of the patchset which might just be the
> final one. I've included the remaining issue Jun'ichi pointed
> out, and added the respective 'reviewed-by' tags where missing.
I've picked these patches up. They are now queued for 3.15 and in
linux-dm.git's 'for-next' branch.
I'll likely rebase linux-dm.git to 3.14 once it is released but aside
from commit id churn there won't be any other changes to this series
(unless linux-next turns something up).
Thanks for sorting this area of mpath out and sticking with this
patchset for so many review iterations.
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv9 0/8] dm-multipath: push back requests instead of queueing
2014-03-12 21:46 ` [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Mike Snitzer
@ 2014-03-13 6:46 ` Hannes Reinecke
0 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-03-13 6:46 UTC (permalink / raw)
To: Mike Snitzer; +Cc: Jun'ichi Nomura, dm-devel, Alasdair Kergon
On 03/12/2014 10:46 PM, Mike Snitzer wrote:
> On Fri, Feb 28 2014 at 9:33am -0500,
> Hannes Reinecke <hare@suse.de> wrote:
>
>> Hi Mike,
>>
>> here's version 9 of the patchset which might just be the
>> final one. I've included the remaining issue Jun'ichi pointed
>> out, and added the respective 'reviewed-by' tags where missing.
>
> I've picked these patches up. They are now queued for 3.15 and in
> linux-dm.git's 'for-next' branch.
>
> I'll likely rebase linux-dm.git to 3.14 once it is released but aside
> from commit id churn there won't be any other changes to this series
> (unless linux-next turns something up).
>
> Thanks for sorting this area of mpath out and sticking with this
> patchset for so many review iterations.
>
Yes! Finally!
Thanks for including it.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-02-28 9:05 ` Junichi Nomura
@ 2014-02-28 9:37 ` Hannes Reinecke
0 siblings, 0 replies; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-28 9:37 UTC (permalink / raw)
To: Junichi Nomura; +Cc: dm-devel, Mike Snitzer, Alasdair Kergon
On 02/28/2014 10:05 AM, Junichi Nomura wrote:
> On 02/27/14 16:30, Hannes Reinecke wrote:
>> @@ -1214,9 +1189,11 @@ static void pg_init_done(void *data, int errors)
>>
>> if (!m->pg_init_required)
>> m->queue_io = 0;
>> -
>> - m->pg_init_delay_retry = delay_retry;
>> - queue_work(kmultipathd, &m->process_queued_ios);
>> + else if (m->current_pg) {
>> + m->pg_init_delay_retry = delay_retry;
>> + if (__pg_init_all_paths(m))
>> + goto out;
>> + }
>
> As I commented before:
> https://www.redhat.com/archives/dm-devel/2014-February/msg00013.html
>
> I think the code here should clear queue_io like this
> when retry was requested but wasn't done:
>
> if (m->pg_init_required) {
> m->pg_init_delay_retry = delay_retry;
> if (__pg_init_all_paths(m))
> goto out;
> }
> m->queue_io = 0;
>
> Otherwise, if __pg_init_all_paths() returns 0, m->queue_io will remain
> as "1" and nobody will clear it.
>
You are right. Will be fixing it up.
Thanks for the review.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-02-27 7:30 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
@ 2014-02-28 9:05 ` Junichi Nomura
2014-02-28 9:37 ` Hannes Reinecke
0 siblings, 1 reply; 17+ messages in thread
From: Junichi Nomura @ 2014-02-28 9:05 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: dm-devel, Mike Snitzer, Alasdair Kergon
On 02/27/14 16:30, Hannes Reinecke wrote:
> @@ -1214,9 +1189,11 @@ static void pg_init_done(void *data, int errors)
>
> if (!m->pg_init_required)
> m->queue_io = 0;
> -
> - m->pg_init_delay_retry = delay_retry;
> - queue_work(kmultipathd, &m->process_queued_ios);
> + else if (m->current_pg) {
> + m->pg_init_delay_retry = delay_retry;
> + if (__pg_init_all_paths(m))
> + goto out;
> + }
As I commented before:
https://www.redhat.com/archives/dm-devel/2014-February/msg00013.html
I think the code here should clear queue_io like this
when retry was requested but wasn't done:
if (m->pg_init_required) {
m->pg_init_delay_retry = delay_retry;
if (__pg_init_all_paths(m))
goto out;
}
m->queue_io = 0;
Otherwise, if __pg_init_all_paths() returns 0, m->queue_io will remain
as "1" and nobody will clear it.
--
Jun'ichi Nomura, NEC Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/8] dm mpath: remove process_queued_ios()
2014-02-27 7:30 [PATCHv8 " Hannes Reinecke
@ 2014-02-27 7:30 ` Hannes Reinecke
2014-02-28 9:05 ` Junichi Nomura
0 siblings, 1 reply; 17+ messages in thread
From: Hannes Reinecke @ 2014-02-27 7:30 UTC (permalink / raw)
To: Alasdair Kergon; +Cc: Jun'ichi Nomura, dm-devel, Mike Snitzer
process_queued_ios() has served 3 functions:
1) select pg and pgpath if none is selected
2) start pg_init if requested
3) dispatch queued IOs when pg is ready
Basically, a call to queue_work(process_queued_ios) can be replaced by
dm_table_run_md_queue_async(), which runs request queue and ends up
calling map_io(), which does 1), 2) and 3).
Exception is when !pg_ready() (which means either pg_init is running or
requested), then multipath_busy() prevents map_io() being called from
request_fn.
If pg_init is running, it should be ok as long as pg_init_done() does
the right thing when pg_init is completed, I.e.: restart pg_init if
!pg_ready() or call dm_table_run_md_queue_async() to kick map_io().
If pg_init is requested, we have to make sure the request is detected
and pg_init will be started. pg_init is requested in 3 places:
a) __choose_pgpath() in map_io()
b) __choose_pgpath() in multipath_ioctl()
c) pg_init retry in pg_init_done()
a) is ok because map_io() calls __pg_init_all_paths(), which does 2).
b) needs a call to __pg_init_all_paths(), which does 2).
c) needs a call to __pg_init_all_paths(), which does 2).
So this patch removes process_queued_ios() and ensures that
__pg_init_all_paths() is called at the appropriate locations.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm-mpath.c | 66 ++++++++++++++++++++-------------------------------
1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 1c6ca5c..7549b78 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -93,8 +93,6 @@ struct multipath {
unsigned pg_init_count; /* Number of times pg_init called */
unsigned pg_init_delay_msecs; /* Number of msecs before pg_init retry */
- struct work_struct process_queued_ios;
-
struct work_struct trigger_event;
/*
@@ -119,7 +117,6 @@ typedef int (*action_fn) (struct pgpath *pgpath);
static struct kmem_cache *_mpio_cache;
static struct workqueue_struct *kmultipathd, *kmpath_handlerd;
-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 int __pgpath_busy(struct pgpath *pgpath);
@@ -197,7 +194,6 @@ static struct multipath *alloc_multipath(struct dm_target *ti)
spin_lock_init(&m->lock);
m->queue_io = 1;
m->pg_init_delay_msecs = DM_PG_INIT_DELAY_DEFAULT;
- INIT_WORK(&m->process_queued_ios, process_queued_ios);
INIT_WORK(&m->trigger_event, trigger_event);
init_waitqueue_head(&m->pg_init_wait);
mutex_init(&m->work_mutex);
@@ -254,16 +250,21 @@ static void clear_mapinfo(struct multipath *m, union map_info *info)
* Path selection
*-----------------------------------------------*/
-static void __pg_init_all_paths(struct multipath *m)
+static int __pg_init_all_paths(struct multipath *m)
{
struct pgpath *pgpath;
unsigned long pg_init_delay = 0;
if (m->pg_init_in_progress || m->pg_init_disabled)
- return;
+ return 0;
m->pg_init_count++;
m->pg_init_required = 0;
+
+ /* Check here to reset pg_init_required */
+ if (!m->current_pg)
+ return 0;
+
if (m->pg_init_delay_retry)
pg_init_delay = msecs_to_jiffies(m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT ?
m->pg_init_delay_msecs : DM_PG_INIT_DELAY_MSECS);
@@ -275,6 +276,7 @@ static void __pg_init_all_paths(struct multipath *m)
pg_init_delay))
m->pg_init_in_progress++;
}
+ return m->pg_init_in_progress;
}
static void __switch_pg(struct multipath *m, struct pgpath *pgpath)
@@ -436,40 +438,13 @@ static int queue_if_no_path(struct multipath *m, unsigned queue_if_no_path,
m->saved_queue_if_no_path = queue_if_no_path;
m->queue_if_no_path = queue_if_no_path;
if (!m->queue_if_no_path)
- queue_work(kmultipathd, &m->process_queued_ios);
+ dm_table_run_md_queue_async(m->ti->table);
spin_unlock_irqrestore(&m->lock, flags);
return 0;
}
-static void process_queued_ios(struct work_struct *work)
-{
- struct multipath *m =
- container_of(work, struct multipath, process_queued_ios);
- struct pgpath *pgpath = NULL;
- unsigned must_queue = 1;
- unsigned long flags;
-
- spin_lock_irqsave(&m->lock, flags);
-
- if (!m->current_pgpath)
- __choose_pgpath(m, 0);
-
- pgpath = m->current_pgpath;
-
- if ((pgpath && !m->queue_io) ||
- (!pgpath && !m->queue_if_no_path))
- must_queue = 0;
-
- if (pgpath && m->pg_init_required)
- __pg_init_all_paths(m);
-
- spin_unlock_irqrestore(&m->lock, flags);
- if (!must_queue)
- dm_table_run_md_queue_async(m->ti->table);
-}
-
/*
* An event is triggered whenever a path is taken out of use.
* Includes path failure and PG bypass.
@@ -1016,7 +991,7 @@ static int reinstate_path(struct pgpath *pgpath)
if (!m->nr_valid_paths++) {
m->current_pgpath = NULL;
- queue_work(kmultipathd, &m->process_queued_ios);
+ dm_table_run_md_queue_async(m->ti->table);
} else if (m->hw_handler_name && (m->current_pg == pgpath->pg)) {
if (queue_work(kmpath_handlerd, &pgpath->activate_path.work))
m->pg_init_in_progress++;
@@ -1214,9 +1189,11 @@ static void pg_init_done(void *data, int errors)
if (!m->pg_init_required)
m->queue_io = 0;
-
- m->pg_init_delay_retry = delay_retry;
- queue_work(kmultipathd, &m->process_queued_ios);
+ else if (m->current_pg) {
+ m->pg_init_delay_retry = delay_retry;
+ if (__pg_init_all_paths(m))
+ goto out;
+ }
/*
* Wake up any thread waiting to suspend.
@@ -1594,8 +1571,17 @@ static int multipath_ioctl(struct dm_target *ti, unsigned int cmd,
r = err;
}
- if (r == -ENOTCONN && !fatal_signal_pending(current))
- queue_work(kmultipathd, &m->process_queued_ios);
+ if (r == -ENOTCONN && !fatal_signal_pending(current)) {
+ spin_lock_irqsave(&m->lock, flags);
+ if (!m->current_pg) {
+ /* Path status changed, redo selection */
+ __choose_pgpath(m, 0);
+ }
+ if (m->pg_init_required)
+ __pg_init_all_paths(m);
+ spin_unlock_irqrestore(&m->lock, flags);
+ dm_table_run_md_queue_async(m->ti->table);
+ }
return r ? : __blkdev_driver_ioctl(bdev, mode, cmd, arg);
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-03-13 6:46 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 14:33 [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Hannes Reinecke
2014-02-28 14:33 ` [PATCH 1/8] dm mpath: do not call pg_init when it is already running Hannes Reinecke
2014-02-28 14:33 ` [PATCH 2/8] dm table: add dm_table_run_md_queue_async Hannes Reinecke
2014-02-28 14:33 ` [PATCH 3/8] dm mpath: push back requests instead of queueing Hannes Reinecke
2014-02-28 14:33 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
2014-03-02 23:12 ` Junichi Nomura
2014-03-05 8:59 ` Hannes Reinecke
2014-03-05 14:24 ` Mike Snitzer
2014-02-28 14:33 ` [PATCH 5/8] dm mpath: reduce memory pressure when requeuing Hannes Reinecke
2014-02-28 14:33 ` [PATCH 6/8] dm mpath: remove map_io() Hannes Reinecke
2014-02-28 14:33 ` [PATCH 7/8] dm mpath: remove extra nesting in map function Hannes Reinecke
2014-02-28 14:33 ` [PATCH 8/8] dm-mpath: do not activate failed paths Hannes Reinecke
2014-03-12 21:46 ` [PATCHv9 0/8] dm-multipath: push back requests instead of queueing Mike Snitzer
2014-03-13 6:46 ` Hannes Reinecke
-- strict thread matches above, loose matches on Subject: below --
2014-02-27 7:30 [PATCHv8 " Hannes Reinecke
2014-02-27 7:30 ` [PATCH 4/8] dm mpath: remove process_queued_ios() Hannes Reinecke
2014-02-28 9:05 ` Junichi Nomura
2014-02-28 9:37 ` Hannes Reinecke
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.