Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Device-mapper Development (http://www.linux-archive.org/device-mapper-development/)
-   -   dm-mpath: separate pg-init handling from process_queued_ios() (http://www.linux-archive.org/device-mapper-development/318278-dm-mpath-separate-pg-init-handling-process_queued_ios.html)

Kiyoshi Ueda 02-01-2010 03:22 AM

dm-mpath: separate pg-init handling from process_queued_ios()
 
This patch is a preparation of the next patch, which fixes the issue
that ioctl isn't processed until any I/O is issued. (And also it is
a preparation of another patch-set to remove multipath internal queue.)
No functional change.

This patch changes pg-init kickers to use activate_path() directly
instead of using process_queued_ios().

process_queued_ios() has been used for pg-init handling, which needs
a kthread context.
However, recent development introduced a special work, activate_path(),
for that purpose. So there is no need to use process_queued_ios().

Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Cc: Alasdair G Kergon <agk@redhat.com>
---
drivers/md/dm-mpath.c | 55 +++++++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 16 deletions(-)

Index: 2.6.33-rc6/drivers/md/dm-mpath.c
================================================== =================
--- 2.6.33-rc6.orig/drivers/md/dm-mpath.c
+++ 2.6.33-rc6/drivers/md/dm-mpath.c
@@ -235,6 +235,21 @@ static void free_multipath(struct multip
* Path selection
*-----------------------------------------------*/

+static void __pg_init(struct multipath *m)
+{
+ struct pgpath *pgpath;
+
+ m->pg_init_count++;
+ m->pg_init_required = 0;
+ list_for_each_entry(pgpath, &m->current_pg->pgpaths, list) {
+ /* Skip failed paths */
+ if (!pgpath->is_active)
+ continue;
+ if (queue_work(kmpath_handlerd, &pgpath->activate_path))
+ m->pg_init_in_progress++;
+ }
+}
+
static void __switch_pg(struct multipath *m, struct pgpath *pgpath)
{
m->current_pg = pgpath->pg;
@@ -350,8 +365,9 @@ static int map_io(struct multipath *m, s
/* Queue for the daemon to resubmit */
list_add_tail(&clone->queuelist, &m->queued_ios);
m->queue_size++;
- if ((m->pg_init_required && !m->pg_init_in_progress) ||
- !m->queue_io)
+ if (m->pg_init_required && !m->pg_init_in_progress && pgpath)
+ __pg_init(m);
+ else if (!m->queue_io)
queue_work(kmultipathd, &m->process_queued_ios);
pgpath = NULL;
r = DM_MAPIO_SUBMITTED;
@@ -439,7 +455,7 @@ static void process_queued_ios(struct wo
{
struct multipath *m =
container_of(work, struct multipath, process_queued_ios);
- struct pgpath *pgpath = NULL, *tmp;
+ struct pgpath *pgpath = NULL;
unsigned must_queue = 1;
unsigned long flags;

@@ -457,17 +473,9 @@ static void process_queued_ios(struct wo
(!pgpath && !m->queue_if_no_path))
must_queue = 0;

- if (m->pg_init_required && !m->pg_init_in_progress && pgpath) {
- m->pg_init_count++;
- m->pg_init_required = 0;
- list_for_each_entry(tmp, &pgpath->pg->pgpaths, list) {
- /* Skip failed paths */
- if (!tmp->is_active)
- continue;
- if (queue_work(kmpath_handlerd, &tmp->activate_path))
- m->pg_init_in_progress++;
- }
- }
+ if (m->pg_init_required && !m->pg_init_in_progress && pgpath)
+ __pg_init(m);
+
out:
spin_unlock_irqrestore(&m->lock, flags);
if (!must_queue)
@@ -1215,9 +1223,24 @@ static void pg_init_done(void *data, int
/* Activations of other paths are still on going */
goto out;

- if (!m->pg_init_required)
- m->queue_io = 0;
+ if (m->pg_init_required) {
+ /* Requested retry or a new pg-init */
+ if (likely(m->current_pgpath)) {
+ __pg_init(m);
+ goto out;
+ }
+
+ /*
+ * The condition requiring pg-init has been changed by someone
+ * after the pg-init had been requested.
+ * Cancel m->pg_init_required here explicitly, and start over
+ * from path selection.
+ */
+ m->pg_init_required = 0;
+ m->current_pg = NULL;
+ }

+ m->queue_io = 0;
queue_work(kmultipathd, &m->process_queued_ios);

/*

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Kiyoshi Ueda 02-04-2010 09:31 AM

dm-mpath: separate pg-init handling from process_queued_ios()
 
Hi Alasdair,

Thanks for your review and comments.

On 02/03/2010 03:24 AM +0900, Alasdair G Kergon wrote:
> On Mon, Feb 01, 2010 at 01:22:23PM +0900, Kiyoshi Ueda wrote:
>> This patch is a preparation of the next patch, which fixes the issue
>> that ioctl isn't processed until any I/O is issued. (And also it is
>> a preparation of another patch-set to remove multipath internal queue.)
>> No functional change.
>
> I've split this into two, but it's very 'tight' code forming a state
> machine here, and while I understand the principle of the patch, I can't
> spot an easy way to tell that all the paths through the code are right.
> Is it worth trying to extend the explanation, or will later patches be
> replacing this code, making it better to wait for that new code before
> reviewing?
>
> ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/dm-mpath-refactor-pg_init-trigger.patch

While trying to extend the explanation, I come to think there may be
better implementation for pg_init refactoring.
So please wait reviewing dm-mpath-refactor-pg_init-trigger.patch until
my next update.
(I think dm-mpath-refactor-pg_init.patch is no problem to apply even
at this point.)

Thanks,
Kiyoshi Ueda

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


All times are GMT. The time now is 05:59 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.