Re: [PATCH 4/5] writeback: per task dirty rate limit

Wu Fengguang
2011-08-07T06:19:45Z

On Sat, Aug 06, 2011 at 10:35:31PM +0800, Andrea Righi wrote:
> On Sat, Aug 06, 2011 at 04:44:51PM +0800, Wu Fengguang wrote:
> > Add two fields to task_struct.
> >
> > 1) account dirtied pages in the individual tasks, for accuracy
> > 2) per-task balance_dirty_pages() call intervals, for flexibility
> >
> > The balance_dirty_pages() call interval (ie. nr_dirtied_pause) will
> > scale near-sqrt to the safety gap between dirty pages and threshold.
> >
> > XXX: The main problem of per-task nr_dirtied is, if 10k tasks start
> > dirtying pages at exactly the same time, each task will be assigned a
> > large initial nr_dirtied_pause, so that the dirty threshold will be
> > exceeded long before each task reached its nr_dirtied_pause and hence
> > call balance_dirty_pages().
> >
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
>
> A minor nitpick below.
>
> Reviewed-by: Andrea Righi <andrea@betterlinux.com>

Thank you.

> > +/*
> > * balance_dirty_pages() must be called by processes which are generating dirty
> > * data. It looks at the number of dirty pages in the machine and will force
> > * the caller to perform writeback if the system is over `vm_dirty_ratio'.
>
> I think we should also fix the comment of balance_dirty_pages(), now
> that it's IO-less for the caller. Maybe something like:
>
> /*
> * balance_dirty_pages() must be called by processes which are generating dirty
> * data. It looks at the number of dirty pages in the machine and will force
> * the caller to wait once crossing the dirty threshold. If we're over
> * `background_thresh' then the writeback threads are woken to perform some
> * writeout.
> */

Good catch! I'll add this change to the next patch:

/*
* balance_dirty_pages() must be called by processes which are generating dirty
* data. It looks at the number of dirty pages in the machine and will force
- * the caller to perform writeback if the system is over `vm_dirty_ratio'.
+ * the caller to wait once crossing the (background_thresh + dirty_thresh) / 2.
* If we're over `background_thresh' then the writeback threads are woken to
* perform some writeout.
*/

Thanks,
Fengguang