Messages in this thread | | | Subject | Re: [patch]block: document blk_plug | From | Shaohua Li <> | Date | Tue, 02 Aug 2011 08:46:10 +0800 |
| |
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.
> 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?
Thanks, Shaohua
| |