[RFC,1/2] dm-mpath: interface change for preparation

Commit Message

Chauhan, Vijay Jan. 30, 2009, 4:42 a.m. UTC

Hi Kiyoshi/Alasdair,

Just a thought. As we are changing __choose_pgpath parameters in this patch, will it be good to use BIO rather than nr_bytes as parameter? Availability of BIO in loadbalancing interface can provide more flexibility for coming up with new load balancing in future.


Thanks,
Vijay

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Kiyoshi Ueda
Sent: Thursday, January 29, 2009 12:40 PM
To: device-mapper development
Subject: [dm-devel] [RFC PATCH 1/2] dm-mpath: interface change for preparation

This patch changes path selector interfaces for service-time oriented dynamic load balancer.

To calculate the service time for an incoming I/O correctly, the load balancer needs the size of the incoming I/O when selecting the next path.


Signed-off-by: Kiyoshi Ueda <[e-mail]>
Signed-off-by: Jun'ichi Nomura <[e-mail]>
---
 drivers/md/dm-mpath.c         |   27 ++++++++++++++++-----------
 drivers/md/dm-path-selector.h |    9 ++++++---
 drivers/md/dm-queue-length.c  |    8 +++++---
 drivers/md/dm-round-robin.c   |    2 +-
 4 files changed, 28 insertions(+), 18 deletions(-)

 }

-static int ql_start_io(struct path_selector *ps, struct dm_path *path)
+static int ql_start_io(struct path_selector *ps, struct dm_path *path,
+                      size_t nr_bytes)
 {
        struct path_info *pi = path->pscontext;

@@ -175,7 +176,8 @@ static int ql_start_io(struct path_selec
        return 0;
 }

-static int ql_end_io(struct path_selector *ps, struct dm_path *path)
+static int ql_end_io(struct path_selector *ps, struct dm_path *path,
+                    size_t nr_bytes)
 {
        struct path_info *pi = path->pscontext;


--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Kiyoshi Ueda Jan. 30, 2009, 8:06 a.m. UTC | #1

Hi Vijay,

Thank you for the comment.

On 01/30/2009 01:42 PM +0900, Chauhan, Vijay wrote:
> Hi Kiyoshi/Alasdair,
> 
> Just a thought. As we are changing __choose_pgpath parameters
> in this patch, will it be good to use BIO rather than nr_bytes
> as parameter? Availability of BIO in loadbalancing interface can
> provide more flexibility for coming up with new load balancing
> in future.

I understand that very much.
But I'd like to keep independency of path-selector from the type
of I/O structure as much as possible, since both bio-based targets
and request-based targets may want to use the same path-selector
in the future.  (e.g. multipath, which is requset-based,
and mirror, which is bio-based.)

So if we need only a few arguments, non-structured parameters
would be good, I think.
Do you have any idea what other parameters in BIO are useful for
load balancing decision?

Thanks,
Kiyoshi Ueda

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Patch

Index: 2.6.29-rc2/drivers/md/dm-mpath.c
===================================================================
--- 2.6.29-rc2.orig/drivers/md/dm-mpath.c
+++ 2.6.29-rc2/drivers/md/dm-mpath.c
@@ -103,6 +103,7 @@  struct multipath {
 struct dm_mpath_io {
        struct pgpath *pgpath;
        struct dm_bio_details details;
+       size_t nr_bytes;
 };

 typedef int (*action_fn) (struct pgpath *pgpath); @@ -251,11 +252,12 @@ static void __switch_pg(struct multipath
        m->pg_init_count = 0;
 }

-static int __choose_path_in_pg(struct multipath *m, struct priority_group *pg)
+static int __choose_path_in_pg(struct multipath *m, struct priority_group *pg,
+                              size_t nr_bytes)
 {
        struct dm_path *path;

-       path = pg->ps.type->select_path(&pg->ps, &m->repeat_count);
+       path = pg->ps.type->select_path(&pg->ps, &m->repeat_count,
+ nr_bytes);
        if (!path)
                return -ENXIO;

@@ -267,7 +269,7 @@  static int __choose_path_in_pg(struct mu
        return 0;
 }

-static void __choose_pgpath(struct multipath *m)
+static void __choose_pgpath(struct multipath *m, size_t nr_bytes)
 {
        struct priority_group *pg;
        unsigned bypassed = 1;
@@ -279,12 +281,12 @@  static void __choose_pgpath(struct multi
        if (m->next_pg) {
                pg = m->next_pg;
                m->next_pg = NULL;
-               if (!__choose_path_in_pg(m, pg))
+               if (!__choose_path_in_pg(m, pg, nr_bytes))
                        return;
        }

        /* Don't change PG until it has no remaining paths */
-       if (m->current_pg && !__choose_path_in_pg(m, m->current_pg))
+       if (m->current_pg && !__choose_path_in_pg(m, m->current_pg,
+ nr_bytes))
                return;

        /*
@@ -296,7 +298,7 @@  static void __choose_pgpath(struct multi
                list_for_each_entry(pg, &m->priority_groups, list) {
                        if (pg->bypassed == bypassed)
                                continue;
-                       if (!__choose_path_in_pg(m, pg))
+                       if (!__choose_path_in_pg(m, pg, nr_bytes))
                                return;
                }
        } while (bypassed--);
@@ -327,6 +329,7 @@  static int map_io(struct multipath *m, s
                  struct dm_mpath_io *mpio, unsigned was_queued)  {
        int r = DM_MAPIO_REMAPPED;
+       size_t nr_bytes = bio->bi_size;
        unsigned long flags;
        struct pgpath *pgpath;

@@ -335,7 +338,7 @@  static int map_io(struct multipath *m, s
        /* Do we need to select a new pgpath? */
        if (!m->current_pgpath ||
            (!m->queue_io && (m->repeat_count && --m->repeat_count == 0)))
-               __choose_pgpath(m);
+               __choose_pgpath(m, nr_bytes);

        pgpath = m->current_pgpath;

@@ -360,9 +363,11 @@  static int map_io(struct multipath *m, s
                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);
+               pgpath->pg->ps.type->start_io(&pgpath->pg->ps, &pgpath->path,
+                                             nr_bytes);

        spin_unlock_irqrestore(&m->lock, flags);

@@ -441,7 +446,7 @@  static void process_queued_ios(struct wo
                goto out;

        if (!m->current_pgpath)
-               __choose_pgpath(m);
+               __choose_pgpath(m, 1 << 19); /* Assume 512 KB */

        pgpath = m->current_pgpath;

@@ -1199,7 +1204,7 @@  static int multipath_end_io(struct dm_ta
        if (pgpath) {
                ps = &pgpath->pg->ps;
                if (ps->type->end_io)
-                       ps->type->end_io(ps, &pgpath->path);
+                       ps->type->end_io(ps, &pgpath->path,
+ mpio->nr_bytes);
        }
        if (r != DM_ENDIO_INCOMPLETE)
                mempool_free(mpio, m->mpio_pool); @@ -1415,7 +1420,7 @@ static int multipath_ioctl(struct dm_tar
        spin_lock_irqsave(&m->lock, flags);

        if (!m->current_pgpath)
-               __choose_pgpath(m);
+               __choose_pgpath(m, 1 << 19); /* Assume 512KB */

        if (m->current_pgpath) {
                bdev = m->current_pgpath->path.dev->bdev;
Index: 2.6.29-rc2/drivers/md/dm-path-selector.h
===================================================================
--- 2.6.29-rc2.orig/drivers/md/dm-path-selector.h
+++ 2.6.29-rc2/drivers/md/dm-path-selector.h
@@ -56,7 +56,8 @@  struct path_selector_type {
         * the path fails.
         */
        struct dm_path *(*select_path) (struct path_selector *ps,
-                                    unsigned *repeat_count);
+                                       unsigned *repeat_count,
+                                       size_t nr_bytes);

        /*
         * Notify the selector that a path has failed.
@@ -75,8 +76,10 @@  struct path_selector_type {
        int (*status) (struct path_selector *ps, struct dm_path *path,
                       status_type_t type, char *result, unsigned int maxlen);

-       int (*start_io) (struct path_selector *ps, struct dm_path *path);
-       int (*end_io) (struct path_selector *ps, struct dm_path *path);
+       int (*start_io) (struct path_selector *ps, struct dm_path *path,
+                        size_t nr_bytes);
+       int (*end_io) (struct path_selector *ps, struct dm_path *path,
+                      size_t nr_bytes);
 };

 /* Register a path selector */
Index: 2.6.29-rc2/drivers/md/dm-round-robin.c
===================================================================
--- 2.6.29-rc2.orig/drivers/md/dm-round-robin.c
+++ 2.6.29-rc2/drivers/md/dm-round-robin.c
@@ -161,7 +161,7 @@  static int rr_reinstate_path(struct path  }

 static struct dm_path *rr_select_path(struct path_selector *ps,
-                                  unsigned *repeat_count)
+                                     unsigned *repeat_count, size_t
+ nr_bytes)
 {
        struct selector *s = (struct selector *) ps->context;
        struct path_info *pi = NULL;
Index: 2.6.29-rc2/drivers/md/dm-queue-length.c
===================================================================
--- 2.6.29-rc2.orig/drivers/md/dm-queue-length.c
+++ 2.6.29-rc2/drivers/md/dm-queue-length.c
@@ -138,7 +138,7 @@  static int ql_reinstate_path(struct path  }

 static struct dm_path *ql_select_path(struct path_selector *ps,
-                                     unsigned *repeat_count)
+                                     unsigned *repeat_count, size_t
+ nr_bytes)
 {
        struct selector *s = (struct selector *) ps->context;
        struct path_info *pi = NULL, *best = NULL; @@ -166,7 +166,8 @@ static struct dm_path *ql_select_path(st
        return best->path;