lkml.org 
[lkml]   [2011]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch]block: document blk_plug
On 2011-08-27 00:25, Andrew Morton wrote:
> On Tue, 02 Aug 2011 08:46:10 +0800
> Shaohua Li <shaohua.li@intel.com> wrote:
>
>> On Fri, 2011-07-29 at 20:14 +0800, Suresh Jayaraman wrote:
>>> On 07/29/2011 08:43 AM, Shaohua Li wrote:
>>>> Andrew Morton is asking to document blk_plug, so here is my attempt.
>>>>
>>>> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
>>>> ---
>>>> include/linux/blkdev.h | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> Index: linux/include/linux/blkdev.h
>>>> ===================================================================
>>>> --- linux.orig/include/linux/blkdev.h 2011-07-29 10:51:29.000000000 +0800
>>>> +++ linux/include/linux/blkdev.h 2011-07-29 11:07:49.000000000 +0800
>>>> @@ -858,6 +858,17 @@ struct request_queue *blk_alloc_queue_no
>>>> extern void blk_put_queue(struct request_queue *);
>>>>
>>>> /*
>>>> + * blk_plug gives each task a request list. Since blk_start_plug() called,
>>>> + * requests from the task will be added to the per-task list and then moved
>>>> + * to global request_queue in a batch way at appropriate time(either
>>>> + * blk_finish_plug() is called or task goes to sleep). blk_plug has some
>>>> + * advantages:
>>>> + * 1. Better request merge. The assumption here is requests from a task have
>>>> + * better chances to be merged.
>>>> + * 2. Better scalability. Requests are moved from per-task list to global
>>>> + * request_queue in a batch way, so the total times grabing global
>>>> + * request_queue lock are reduced.
>>>> + *
>>>
>>> Hi Shaohua,
>>>
>>> This seems too brief atleast for someone like me who has not spent much
>>> time with the code and also is not in kerneldoc format. Here's my attempt:
>> Hi Suresh,
>> I like the blk_start_plug part below. The blk_plug part needs more
>> description to explain why we need it.
>>
>
> I'm getting all excited about getting some blk_plug documentation!
>
>>> From: Suresh Jayaraman <sjayaraman@suse.de>
>>> Subject: [PATCH] block: document blk-plug
>>>
>>> Thus spake Andrew Morton:
>>>
>>> "And I have the usual maintainability whine. If someone comes up to
>>> vmscan.c and sees it calling blk_start_plug(), how are they supposed to
>>> work out why that call is there? They go look at the blk_start_plug()
>>> definition and it is undocumented. I think we can do better than this?"
>>>
>>> Shaohua Li attempted to document it. But, I think it was too brief and
>>> was not in kerneldoc format. Here's my attempt to document it.
>>>
>>> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
>>> ---
>>>
>>> block/blk-core.c | 10 ++++++++++
>>> include/linux/blkdev.h | 13 ++++++++-----
>>> 2 files changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>> index b850bed..355aa2c 100644
>>> --- a/block/blk-core.c
>>> +++ b/block/blk-core.c
>>> @@ -2620,6 +2620,16 @@ EXPORT_SYMBOL(kblockd_schedule_delayed_work);
>>>
>>> #define PLUG_MAGIC 0x91827364
>>>
>>> +/**
>>> + * blk_start_plug - initialize blk_plug and track it inside the task_struct
>>> + * @plug: The &struct blk_plug that needs to be initialized
>>> + *
>>> + * Description:
>>> + * Tracking blk_plug inside the task_struct will help with flushing the
>>> + * pending I/O should the task end up blocking between blk_start_plug() and
>>> + * blk_finish_plug() and is important for deadlock avoidance and for the
>>> + * performance.
>>> + */
>> I'm not aware blk_plug is to avoid deadlock. It's most for performance
>> to me. Jens, any idea?
>
> Only we can't document it because we don't understand it. Great.
>
> c'mon Jens. Talk to us?

The pointer in the task_struct is to be able to auto-flush the plug if
the task inadvertently ends up scheduling with requests plugged. For
instance, if we end up doing a wait for a page that is already plugged,
then we must ensure that everything up to that point has been flushed
out.

--
Jens Axboe



\
 
 \ /
  Last update: 2011-08-27 08:39    [W:0.058 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site