Re: [PATCH] block: flush queued bios when the process blocks

2014-05-29 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

> On 2014-05-27 10:26, Mikulas Patocka wrote:
> > On Tue, 27 May 2014, Jens Axboe wrote:
> > 
> > > On 2014-05-27 09:23, Mikulas Patocka wrote:
> > > 
> > > > The patch adds bio list flushing to the scheduler just besides plug
> > > > flushsing.
> > > 
> > > ... which is exactly why I'm commenting. It'd be great to avoid yet one
> > > more
> > > scheduler hook for this sort of thing.
> > > 
> > > --
> > > Jens Axboe
> > 
> > One could create something like schedule notifier chain, but I'm not sure
> > if it is worth the complexity because of just two users. If more users
> > come in the future, it could be generalized.
> 
> Except such a thing already exists, there are unplug callback chains. All I'm
> asking is that you look into how feasible it would be to use something like
> that, instead of reinventing the wheel.
> 
> -- 
> Jens Axboe


You can use this patch as an example that moves current->bio_list to 
struct plug, but I don't recommend to put it in the kernel - this patch 
still has some issues (some lvm raid tests fail) - and at -rc7 stage we 
should really be fixing bugs and not rearchitecting the code.


You should commit my original patch because it is small and it generated 
no regressions for me. Think about stable kernels and enterprise 
distributions - the smaller the patch is, the easier it is to backport.


Mikulas


---
 block/blk-core.c   |   19 ---
 drivers/md/dm-bufio.c  |2 +-
 drivers/md/raid1.c |6 +++---
 drivers/md/raid10.c|6 +++---
 fs/bio.c   |   21 +
 include/linux/blkdev.h |7 +--
 include/linux/sched.h  |4 
 kernel/sched/core.c|7 ---
 8 files changed, 33 insertions(+), 39 deletions(-)

Index: linux-3.15-rc5/block/blk-core.c
===
--- linux-3.15-rc5.orig/block/blk-core.c2014-05-29 23:06:29.0 
+0200
+++ linux-3.15-rc5/block/blk-core.c 2014-05-30 00:30:41.0 +0200
@@ -1828,7 +1828,7 @@ end_io:
  */
 void generic_make_request(struct bio *bio)
 {
-   struct bio_list bio_list_on_stack;
+   struct blk_plug plug;
 
if (!generic_make_request_checks(bio))
return;
@@ -1858,8 +1858,8 @@ void generic_make_request(struct bio *bi
 * it is non-NULL, then a make_request is active, and new requests
 * should be added at the tail
 */
-   if (current->bio_list) {
-   bio_list_add(current->bio_list, bio);
+   if (current->plug) {
+   bio_list_add(>plug->bio_list, bio);
return;
}
 
@@ -1877,17 +1877,18 @@ void generic_make_request(struct bio *bi
 * of the top of the list (no pretending) and so remove it from
 * bio_list, and call into ->make_request() again.
 */
+   blk_start_plug();
+   current->plug->in_generic_make_request = 1;
BUG_ON(bio->bi_next);
-   bio_list_init(_list_on_stack);
-   current->bio_list = _list_on_stack;
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 
q->make_request_fn(q, bio);
 
-   bio = bio_list_pop(current->bio_list);
+   bio = bio_list_pop(>plug->bio_list);
} while (bio);
-   current->bio_list = NULL; /* deactivate */
+   current->plug->in_generic_make_request = 0;
+   blk_finish_plug();
 }
 EXPORT_SYMBOL(generic_make_request);
 
@@ -2965,6 +2966,8 @@ void blk_start_plug(struct blk_plug *plu
INIT_LIST_HEAD(>list);
INIT_LIST_HEAD(>mq_list);
INIT_LIST_HEAD(>cb_list);
+   bio_list_init(>bio_list);
+   plug->in_generic_make_request = 0;
 
/*
 * If this is a nested plug, don't actually assign it. It will be
@@ -3060,6 +3063,8 @@ void blk_flush_plug_list(struct blk_plug
 
BUG_ON(plug->magic != PLUG_MAGIC);
 
+   blk_flush_bio_list(plug);
+
flush_plug_callbacks(plug, from_schedule);
 
if (!list_empty(>mq_list))
Index: linux-3.15-rc5/include/linux/blkdev.h
===
--- linux-3.15-rc5.orig/include/linux/blkdev.h  2014-05-29 23:05:46.0 
+0200
+++ linux-3.15-rc5/include/linux/blkdev.h   2014-05-30 00:30:54.0 
+0200
@@ -1061,6 +1061,8 @@ struct blk_plug {
struct list_head list; /* requests */
struct list_head mq_list; /* blk-mq requests */
struct list_head cb_list; /* md requires an unplug callback */
+   struct bio_list bio_list; /* list of queued bios */
+   int in_generic_make_request;
 };
 #define BLK_MAX_REQUEST_COUNT 16
 
@@ -1100,10 +1102,11 @@ static inline bool blk_needs_flush_plug(
return plug &&
(!list_empty(>list) ||
 !list_empty(>mq_list) ||
-!list_empty(>cb_list));
+!list_empty(>cb_list) ||
+

Re: [PATCH] block: flush queued bios when the process blocks

2014-05-29 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

 On 2014-05-27 10:26, Mikulas Patocka wrote:
  On Tue, 27 May 2014, Jens Axboe wrote:
  
   On 2014-05-27 09:23, Mikulas Patocka wrote:
   
The patch adds bio list flushing to the scheduler just besides plug
flushsing.
   
   ... which is exactly why I'm commenting. It'd be great to avoid yet one
   more
   scheduler hook for this sort of thing.
   
   --
   Jens Axboe
  
  One could create something like schedule notifier chain, but I'm not sure
  if it is worth the complexity because of just two users. If more users
  come in the future, it could be generalized.
 
 Except such a thing already exists, there are unplug callback chains. All I'm
 asking is that you look into how feasible it would be to use something like
 that, instead of reinventing the wheel.
 
 -- 
 Jens Axboe


You can use this patch as an example that moves current-bio_list to 
struct plug, but I don't recommend to put it in the kernel - this patch 
still has some issues (some lvm raid tests fail) - and at -rc7 stage we 
should really be fixing bugs and not rearchitecting the code.


You should commit my original patch because it is small and it generated 
no regressions for me. Think about stable kernels and enterprise 
distributions - the smaller the patch is, the easier it is to backport.


Mikulas


---
 block/blk-core.c   |   19 ---
 drivers/md/dm-bufio.c  |2 +-
 drivers/md/raid1.c |6 +++---
 drivers/md/raid10.c|6 +++---
 fs/bio.c   |   21 +
 include/linux/blkdev.h |7 +--
 include/linux/sched.h  |4 
 kernel/sched/core.c|7 ---
 8 files changed, 33 insertions(+), 39 deletions(-)

Index: linux-3.15-rc5/block/blk-core.c
===
--- linux-3.15-rc5.orig/block/blk-core.c2014-05-29 23:06:29.0 
+0200
+++ linux-3.15-rc5/block/blk-core.c 2014-05-30 00:30:41.0 +0200
@@ -1828,7 +1828,7 @@ end_io:
  */
 void generic_make_request(struct bio *bio)
 {
-   struct bio_list bio_list_on_stack;
+   struct blk_plug plug;
 
if (!generic_make_request_checks(bio))
return;
@@ -1858,8 +1858,8 @@ void generic_make_request(struct bio *bi
 * it is non-NULL, then a make_request is active, and new requests
 * should be added at the tail
 */
-   if (current-bio_list) {
-   bio_list_add(current-bio_list, bio);
+   if (current-plug) {
+   bio_list_add(current-plug-bio_list, bio);
return;
}
 
@@ -1877,17 +1877,18 @@ void generic_make_request(struct bio *bi
 * of the top of the list (no pretending) and so remove it from
 * bio_list, and call into -make_request() again.
 */
+   blk_start_plug(plug);
+   current-plug-in_generic_make_request = 1;
BUG_ON(bio-bi_next);
-   bio_list_init(bio_list_on_stack);
-   current-bio_list = bio_list_on_stack;
do {
struct request_queue *q = bdev_get_queue(bio-bi_bdev);
 
q-make_request_fn(q, bio);
 
-   bio = bio_list_pop(current-bio_list);
+   bio = bio_list_pop(current-plug-bio_list);
} while (bio);
-   current-bio_list = NULL; /* deactivate */
+   current-plug-in_generic_make_request = 0;
+   blk_finish_plug(plug);
 }
 EXPORT_SYMBOL(generic_make_request);
 
@@ -2965,6 +2966,8 @@ void blk_start_plug(struct blk_plug *plu
INIT_LIST_HEAD(plug-list);
INIT_LIST_HEAD(plug-mq_list);
INIT_LIST_HEAD(plug-cb_list);
+   bio_list_init(plug-bio_list);
+   plug-in_generic_make_request = 0;
 
/*
 * If this is a nested plug, don't actually assign it. It will be
@@ -3060,6 +3063,8 @@ void blk_flush_plug_list(struct blk_plug
 
BUG_ON(plug-magic != PLUG_MAGIC);
 
+   blk_flush_bio_list(plug);
+
flush_plug_callbacks(plug, from_schedule);
 
if (!list_empty(plug-mq_list))
Index: linux-3.15-rc5/include/linux/blkdev.h
===
--- linux-3.15-rc5.orig/include/linux/blkdev.h  2014-05-29 23:05:46.0 
+0200
+++ linux-3.15-rc5/include/linux/blkdev.h   2014-05-30 00:30:54.0 
+0200
@@ -1061,6 +1061,8 @@ struct blk_plug {
struct list_head list; /* requests */
struct list_head mq_list; /* blk-mq requests */
struct list_head cb_list; /* md requires an unplug callback */
+   struct bio_list bio_list; /* list of queued bios */
+   int in_generic_make_request;
 };
 #define BLK_MAX_REQUEST_COUNT 16
 
@@ -1100,10 +1102,11 @@ static inline bool blk_needs_flush_plug(
return plug 
(!list_empty(plug-list) ||
 !list_empty(plug-mq_list) ||
-!list_empty(plug-cb_list));
+!list_empty(plug-cb_list) ||
+!bio_list_empty(plug-bio_list));
 }
 

Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Kent Overstreet
On Tue, May 27, 2014 at 03:56:00PM -0400, Mikulas Patocka wrote:
> 
> 
> On Tue, 27 May 2014, Jens Axboe wrote:
> 
> > On 2014-05-27 10:26, Mikulas Patocka wrote:
> > > On Tue, 27 May 2014, Jens Axboe wrote:
> > > 
> > > > On 2014-05-27 09:23, Mikulas Patocka wrote:
> > > > 
> > > > > The patch adds bio list flushing to the scheduler just besides plug
> > > > > flushsing.
> > > > 
> > > > ... which is exactly why I'm commenting. It'd be great to avoid yet one
> > > > more
> > > > scheduler hook for this sort of thing.
> > > > 
> > > > --
> > > > Jens Axboe
> > > 
> > > One could create something like schedule notifier chain, but I'm not sure
> > > if it is worth the complexity because of just two users. If more users
> > > come in the future, it could be generalized.
> > 
> > Except such a thing already exists, there are unplug callback chains. All 
> > I'm
> > asking is that you look into how feasible it would be to use something like
> > that, instead of reinventing the wheel.
> > 
> > -- 
> > Jens Axboe
> 
> Do you mean moving current->bio_list to struct blk_plug and calling 
> blk_start_plug/blk_finish_plug around generic_make_request?
> 
> It would be possible on a condition that we can redirect all bios to a 
> workqueue (i.e. eliminate bio_kmalloc and always use bio_alloc_bioset).
> 
> What are performance implications of this - does it make sense to have 
> blk_start_plug/blk_finish_plug around every call to generic_make_request? 
> - that means that all i/o requests will be added to a plug and then 
> unplugged.

We've already got blk_start_plug() calls around IO submission at higher points
in the stack. (I actually have seen it show up in profiles though, it probably
would be worth inlining and slimming down a bit).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

> On 2014-05-27 10:26, Mikulas Patocka wrote:
> > On Tue, 27 May 2014, Jens Axboe wrote:
> > 
> > > On 2014-05-27 09:23, Mikulas Patocka wrote:
> > > 
> > > > The patch adds bio list flushing to the scheduler just besides plug
> > > > flushsing.
> > > 
> > > ... which is exactly why I'm commenting. It'd be great to avoid yet one
> > > more
> > > scheduler hook for this sort of thing.
> > > 
> > > --
> > > Jens Axboe
> > 
> > One could create something like schedule notifier chain, but I'm not sure
> > if it is worth the complexity because of just two users. If more users
> > come in the future, it could be generalized.
> 
> Except such a thing already exists, there are unplug callback chains. All I'm
> asking is that you look into how feasible it would be to use something like
> that, instead of reinventing the wheel.
> 
> -- 
> Jens Axboe

Do you mean moving current->bio_list to struct blk_plug and calling 
blk_start_plug/blk_finish_plug around generic_make_request?

It would be possible on a condition that we can redirect all bios to a 
workqueue (i.e. eliminate bio_kmalloc and always use bio_alloc_bioset).

What are performance implications of this - does it make sense to have 
blk_start_plug/blk_finish_plug around every call to generic_make_request? 
- that means that all i/o requests will be added to a plug and then 
unplugged.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Kent Overstreet
On Tue, May 27, 2014 at 8:08 AM, Jens Axboe  wrote:
> This really begs the question of why we just don't use the per-process plugs
> for this. We already have scheduler hooks in place to flush those at the
> appropriate time. Why are we reinventing something for essentially the same
> thing?

Yes! Unifying the two plugging mechanisms has been on my todo/wishlist for ages.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 10:26, Mikulas Patocka wrote:

On Tue, 27 May 2014, Jens Axboe wrote:


On 2014-05-27 09:23, Mikulas Patocka wrote:


The patch adds bio list flushing to the scheduler just besides plug
flushsing.


... which is exactly why I'm commenting. It'd be great to avoid yet one more
scheduler hook for this sort of thing.

--
Jens Axboe


One could create something like schedule notifier chain, but I'm not sure
if it is worth the complexity because of just two users. If more users
come in the future, it could be generalized.


Except such a thing already exists, there are unplug callback chains. 
All I'm asking is that you look into how feasible it would be to use 
something like that, instead of reinventing the wheel.


--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka
On Tue, 27 May 2014, Jens Axboe wrote:

> On 2014-05-27 09:23, Mikulas Patocka wrote:
> 
> > The patch adds bio list flushing to the scheduler just besides plug
> > flushsing.
> 
> ... which is exactly why I'm commenting. It'd be great to avoid yet one more
> scheduler hook for this sort of thing.
> 
> -- 
> Jens Axboe

One could create something like schedule notifier chain, but I'm not sure 
if it is worth the complexity because of just two users. If more users 
come in the future, it could be generalized.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 09:23, Mikulas Patocka wrote:



On Tue, 27 May 2014, Jens Axboe wrote:


On 2014-05-27 09:03, Mikulas Patocka wrote:

The block layer uses per-process bio list to avoid recursion in
generic_make_request. When generic_make_request is called recursively, the
bio is added to current->bio_list and the function returns immediatelly.
The top-level instance of generic_make_requests takes bios from
current->bio_list and processes them.


This really begs the question of why we just don't use the per-process plugs
for this. We already have scheduler hooks in place to flush those at the
appropriate time. Why are we reinventing something for essentially the same
thing?

--
Jens Axboe


Plugs work with requests, this patch works with bios. They are different
structures, so you can't use one infrastructure to process them.


Yes... I realize the list and plugs are for requests. But there's 
nothing preventing a non-rq hook, we have uses like that too. And it 
could easily be extended to handle bio lists, too.



The patch adds bio list flushing to the scheduler just besides plug
flushsing.


... which is exactly why I'm commenting. It'd be great to avoid yet one 
more scheduler hook for this sort of thing.


--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

> On 2014-05-27 09:03, Mikulas Patocka wrote:
> > The block layer uses per-process bio list to avoid recursion in
> > generic_make_request. When generic_make_request is called recursively, the
> > bio is added to current->bio_list and the function returns immediatelly.
> > The top-level instance of generic_make_requests takes bios from
> > current->bio_list and processes them.
> 
> This really begs the question of why we just don't use the per-process plugs
> for this. We already have scheduler hooks in place to flush those at the
> appropriate time. Why are we reinventing something for essentially the same
> thing?
> 
> -- 
> Jens Axboe

Plugs work with requests, this patch works with bios. They are different 
structures, so you can't use one infrastructure to process them.

The patch adds bio list flushing to the scheduler just besides plug 
flushsing.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 09:03, Mikulas Patocka wrote:

The block layer uses per-process bio list to avoid recursion in
generic_make_request. When generic_make_request is called recursively, the
bio is added to current->bio_list and the function returns immediatelly.
The top-level instance of generic_make_requests takes bios from
current->bio_list and processes them.


This really begs the question of why we just don't use the per-process 
plugs for this. We already have scheduler hooks in place to flush those 
at the appropriate time. Why are we reinventing something for 
essentially the same thing?


--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 09:03, Mikulas Patocka wrote:

The block layer uses per-process bio list to avoid recursion in
generic_make_request. When generic_make_request is called recursively, the
bio is added to current-bio_list and the function returns immediatelly.
The top-level instance of generic_make_requests takes bios from
current-bio_list and processes them.


This really begs the question of why we just don't use the per-process 
plugs for this. We already have scheduler hooks in place to flush those 
at the appropriate time. Why are we reinventing something for 
essentially the same thing?


--
Jens Axboe

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

 On 2014-05-27 09:03, Mikulas Patocka wrote:
  The block layer uses per-process bio list to avoid recursion in
  generic_make_request. When generic_make_request is called recursively, the
  bio is added to current-bio_list and the function returns immediatelly.
  The top-level instance of generic_make_requests takes bios from
  current-bio_list and processes them.
 
 This really begs the question of why we just don't use the per-process plugs
 for this. We already have scheduler hooks in place to flush those at the
 appropriate time. Why are we reinventing something for essentially the same
 thing?
 
 -- 
 Jens Axboe

Plugs work with requests, this patch works with bios. They are different 
structures, so you can't use one infrastructure to process them.

The patch adds bio list flushing to the scheduler just besides plug 
flushsing.

Mikulas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 09:23, Mikulas Patocka wrote:



On Tue, 27 May 2014, Jens Axboe wrote:


On 2014-05-27 09:03, Mikulas Patocka wrote:

The block layer uses per-process bio list to avoid recursion in
generic_make_request. When generic_make_request is called recursively, the
bio is added to current-bio_list and the function returns immediatelly.
The top-level instance of generic_make_requests takes bios from
current-bio_list and processes them.


This really begs the question of why we just don't use the per-process plugs
for this. We already have scheduler hooks in place to flush those at the
appropriate time. Why are we reinventing something for essentially the same
thing?

--
Jens Axboe


Plugs work with requests, this patch works with bios. They are different
structures, so you can't use one infrastructure to process them.


Yes... I realize the list and plugs are for requests. But there's 
nothing preventing a non-rq hook, we have uses like that too. And it 
could easily be extended to handle bio lists, too.



The patch adds bio list flushing to the scheduler just besides plug
flushsing.


... which is exactly why I'm commenting. It'd be great to avoid yet one 
more scheduler hook for this sort of thing.


--
Jens Axboe

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka
On Tue, 27 May 2014, Jens Axboe wrote:

 On 2014-05-27 09:23, Mikulas Patocka wrote:
 
  The patch adds bio list flushing to the scheduler just besides plug
  flushsing.
 
 ... which is exactly why I'm commenting. It'd be great to avoid yet one more
 scheduler hook for this sort of thing.
 
 -- 
 Jens Axboe

One could create something like schedule notifier chain, but I'm not sure 
if it is worth the complexity because of just two users. If more users 
come in the future, it could be generalized.

Mikulas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Jens Axboe

On 2014-05-27 10:26, Mikulas Patocka wrote:

On Tue, 27 May 2014, Jens Axboe wrote:


On 2014-05-27 09:23, Mikulas Patocka wrote:


The patch adds bio list flushing to the scheduler just besides plug
flushsing.


... which is exactly why I'm commenting. It'd be great to avoid yet one more
scheduler hook for this sort of thing.

--
Jens Axboe


One could create something like schedule notifier chain, but I'm not sure
if it is worth the complexity because of just two users. If more users
come in the future, it could be generalized.


Except such a thing already exists, there are unplug callback chains. 
All I'm asking is that you look into how feasible it would be to use 
something like that, instead of reinventing the wheel.


--
Jens Axboe

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Kent Overstreet
On Tue, May 27, 2014 at 8:08 AM, Jens Axboe ax...@kernel.dk wrote:
 This really begs the question of why we just don't use the per-process plugs
 for this. We already have scheduler hooks in place to flush those at the
 appropriate time. Why are we reinventing something for essentially the same
 thing?

Yes! Unifying the two plugging mechanisms has been on my todo/wishlist for ages.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Mikulas Patocka


On Tue, 27 May 2014, Jens Axboe wrote:

 On 2014-05-27 10:26, Mikulas Patocka wrote:
  On Tue, 27 May 2014, Jens Axboe wrote:
  
   On 2014-05-27 09:23, Mikulas Patocka wrote:
   
The patch adds bio list flushing to the scheduler just besides plug
flushsing.
   
   ... which is exactly why I'm commenting. It'd be great to avoid yet one
   more
   scheduler hook for this sort of thing.
   
   --
   Jens Axboe
  
  One could create something like schedule notifier chain, but I'm not sure
  if it is worth the complexity because of just two users. If more users
  come in the future, it could be generalized.
 
 Except such a thing already exists, there are unplug callback chains. All I'm
 asking is that you look into how feasible it would be to use something like
 that, instead of reinventing the wheel.
 
 -- 
 Jens Axboe

Do you mean moving current-bio_list to struct blk_plug and calling 
blk_start_plug/blk_finish_plug around generic_make_request?

It would be possible on a condition that we can redirect all bios to a 
workqueue (i.e. eliminate bio_kmalloc and always use bio_alloc_bioset).

What are performance implications of this - does it make sense to have 
blk_start_plug/blk_finish_plug around every call to generic_make_request? 
- that means that all i/o requests will be added to a plug and then 
unplugged.

Mikulas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] block: flush queued bios when the process blocks

2014-05-27 Thread Kent Overstreet
On Tue, May 27, 2014 at 03:56:00PM -0400, Mikulas Patocka wrote:
 
 
 On Tue, 27 May 2014, Jens Axboe wrote:
 
  On 2014-05-27 10:26, Mikulas Patocka wrote:
   On Tue, 27 May 2014, Jens Axboe wrote:
   
On 2014-05-27 09:23, Mikulas Patocka wrote:

 The patch adds bio list flushing to the scheduler just besides plug
 flushsing.

... which is exactly why I'm commenting. It'd be great to avoid yet one
more
scheduler hook for this sort of thing.

--
Jens Axboe
   
   One could create something like schedule notifier chain, but I'm not sure
   if it is worth the complexity because of just two users. If more users
   come in the future, it could be generalized.
  
  Except such a thing already exists, there are unplug callback chains. All 
  I'm
  asking is that you look into how feasible it would be to use something like
  that, instead of reinventing the wheel.
  
  -- 
  Jens Axboe
 
 Do you mean moving current-bio_list to struct blk_plug and calling 
 blk_start_plug/blk_finish_plug around generic_make_request?
 
 It would be possible on a condition that we can redirect all bios to a 
 workqueue (i.e. eliminate bio_kmalloc and always use bio_alloc_bioset).
 
 What are performance implications of this - does it make sense to have 
 blk_start_plug/blk_finish_plug around every call to generic_make_request? 
 - that means that all i/o requests will be added to a plug and then 
 unplugged.

We've already got blk_start_plug() calls around IO submission at higher points
in the stack. (I actually have seen it show up in profiles though, it probably
would be worth inlining and slimming down a bit).
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/