linux-kernel - Re: [PATCH 04/10] block: initial patch for on-stack per-task plugging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1300430212.2337.141.camel@sli10-conroe>
Date:	Fri, 18 Mar 2011 14:36:52 +0800
From:	Shaohua Li <shaohua.li@...el.com>
To:	Jens Axboe <jaxboe@...ionio.com>
Cc:	Vivek Goyal <vgoyal@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hch@...radead.org" <hch@...radead.org>,
	"jmoyer@...hat.com" <jmoyer@...hat.com>
Subject: Re: [PATCH 04/10] block: initial patch for on-stack per-task
 plugging

On Thu, 2011-03-17 at 17:43 +0800, Jens Axboe wrote:
> On 2011-03-17 02:00, Shaohua Li wrote:
> > On Thu, 2011-03-17 at 01:31 +0800, Vivek Goyal wrote:
> >> On Wed, Mar 16, 2011 at 04:18:30PM +0800, Shaohua Li wrote:
> >>> 2011/1/22 Jens Axboe <jaxboe@...ionio.com>:
> >>>> Signed-off-by: Jens Axboe <jaxboe@...ionio.com>
> >>>> ---
> >>>>  block/blk-core.c          |  357 ++++++++++++++++++++++++++++++++------------
> >>>>  block/elevator.c          |    6 +-
> >>>>  include/linux/blk_types.h |    2 +
> >>>>  include/linux/blkdev.h    |   30 ++++
> >>>>  include/linux/elevator.h  |    1 +
> >>>>  include/linux/sched.h     |    6 +
> >>>>  kernel/exit.c             |    1 +
> >>>>  kernel/fork.c             |    3 +
> >>>>  kernel/sched.c            |   11 ++-
> >>>>  9 files changed, 317 insertions(+), 100 deletions(-)
> >>>>
> >>>> diff --git a/block/blk-core.c b/block/blk-core.c
> >>>> index 960f12c..42dbfcc 100644
> >>>> --- a/block/blk-core.c
> >>>> +++ b/block/blk-core.c
> >>>> @@ -27,6 +27,7 @@
> >>>>  #include <linux/writeback.h>
> >>>>  #include <linux/task_io_accounting_ops.h>
> >>>>  #include <linux/fault-inject.h>
> >>>> +#include <linux/list_sort.h>
> >>>>
> >>>>  #define CREATE_TRACE_POINTS
> >>>>  #include <trace/events/block.h>
> >>>> @@ -213,7 +214,7 @@ static void blk_delay_work(struct work_struct *work)
> >>>>
> >>>>        q = container_of(work, struct request_queue, delay_work.work);
> >>>>        spin_lock_irq(q->queue_lock);
> >>>> -       q->request_fn(q);
> >>>> +       __blk_run_queue(q);
> >>>>        spin_unlock_irq(q->queue_lock);
> >>>>  }
> >>> Hi Jens,
> >>> I have some questions about the per-task plugging. Since the request
> >>> list is per-task, and each task delivers its requests at finish flush
> >>> or schedule. But when one cpu delivers requests to global queue, other
> >>> cpus don't know. This seems to have problem. For example:
> >>> 1. get_request_wait() can only flush current task's request list,
> >>> other cpus/tasks might still have a lot of requests, which aren't sent
> >>> to request_queue.
> >>
> >> But very soon these requests will be sent to request queue as soon task
> >> is either scheduled out or task explicitly flushes the plug? So we might
> >> wait a bit longer but that might not matter in general, i guess. 
> > Yes, I understand there is just a bit delay. I don't know how severe it
> > is, but this still could be a problem, especially for fast storage or
> > random I/O. My current tests show slight regression (3% or so) with
> > Jens's for 2.6.39/core branch. I'm still checking if it's caused by the
> > per-task plug, but the per-task plug is highly suspected.
> 
> To check this particular case, you can always just bump the request
> limit. What test is showing a slowdown? 
this is a simple multi-threaded seq read. The issue tends to be request
merge related (not verified yet). The merge reduces about 60% with stack
plug from fio reported data. From trace, without stack plug, requests
from different threads get merged. But with it, such merge is impossible
because flush_plug doesn't check merge, I thought we need add it again.

Thanks,
Shaohua

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