lkml.org 
[lkml]   [2009]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] Initialize mempool and elevator only for request-based dm devices
On Sat, Aug 08 2009 at 12:56am -0400,
Nikanth Karthikesan <knikanth@suse.de> wrote:

> Intialize the request_queue and elevator only when the device is marked as
> a request-based device. This avoids unnecessary creation of mempool for
> requests. Also we wrongly initialize the elevator even for bio-based devices.
> As the /sys/block/dm-*/queue/scheduler is exported for device-mapper devices,
> it is possible to confuse with scheduler options for bio-based devices where
> scheduler is not at all used.
>
> Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

I like how this clearly splits out request-based DM queue initialization.

More comments below.

> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8a311ea..b01dfbe 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2203,7 +2199,25 @@ int dm_swap_table(struct mapped_device *md, struct dm_table *table)
> goto out;
> }
>
> - __unbind(md);
> + if (md->map)
> + __unbind(md);
> + else {
> + /* new device is being marked as either request-based or bio-based */
> + if (dm_table_request_based(table)) {
> + /* Initialize queue for request-based dm */
> + r = blk_init_allocated_queue(md->queue, dm_request_fn,
> + NULL);
> + if (r)
> + goto out;
> + md->saved_make_request_fn = md->queue->make_request_fn;
> + blk_queue_make_request(md->queue, dm_request);

This blk_queue_make_request() is redundant as it was already established
in alloc_dev(). The fact that its behavior is now altered as a result
of the md->saved_make_request_fn assignment doesn't change the fact that
the queue will be using dm_request(). Am I missing something?

Also, I think the code should be cleaned up as follows:

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index b01dfbe..1b8aa43 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2199,25 +2199,18 @@ int dm_swap_table(struct mapped_device *md, struct dm_table *table)
goto out;
}

- if (md->map)
- __unbind(md);
- else {
- /* new device is being marked as either request-based or bio-based */
- if (dm_table_request_based(table)) {
- /* Initialize queue for request-based dm */
- r = blk_init_allocated_queue(md->queue, dm_request_fn,
- NULL);
- if (r)
- goto out;
- md->saved_make_request_fn = md->queue->make_request_fn;
- blk_queue_make_request(md->queue, dm_request);
- blk_queue_softirq_done(md->queue, dm_softirq_done);
- blk_queue_prep_rq(md->queue, dm_prep_fn);
- blk_queue_lld_busy(md->queue, dm_lld_busy);
-
- }
+ if (!md->map && dm_table_request_based(table)) {
+ /* Initialize queue for request-based dm */
+ r = blk_init_allocated_queue(md->queue, dm_request_fn, NULL);
+ if (r)
+ goto out;
+ md->saved_make_request_fn = md->queue->make_request_fn;
+ blk_queue_softirq_done(md->queue, dm_softirq_done);
+ blk_queue_prep_rq(md->queue, dm_prep_fn);
+ blk_queue_lld_busy(md->queue, dm_lld_busy);
}

+ __unbind(md);
r = __bind(md, table, &limits);

out:

\
 
 \ /
  Last update: 2009-08-08 18:25    [W:0.331 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site