lkml.org 
[lkml]   [2011]   [Aug]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 4/5] writeback: per task dirty rate limit
    On Mon, Aug 08, 2011 at 10:23:18PM +0800, Wu Fengguang wrote:
    > On Mon, Aug 08, 2011 at 09:47:14PM +0800, Peter Zijlstra wrote:
    > > On Sat, 2011-08-06 at 16:44 +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>
    > > > ---
    > > > include/linux/sched.h | 7 ++
    > > > mm/memory_hotplug.c | 3 -
    > > > mm/page-writeback.c | 106 +++++++++-------------------------------
    > > > 3 files changed, 32 insertions(+), 84 deletions(-)
    > >
    > > No fork() hooks? This way tasks inherit their parent's dirty count on
    > > clone().
    >
    > btw, I do have another patch queued for improving the "leaked dirties
    > on exit" case :)
    >
    > Thanks,
    > Fengguang
    > ---
    > Subject: writeback: charge leaked page dirties to active tasks
    > Date: Tue Apr 05 13:21:19 CST 2011
    >
    > It's a years long problem that a large number of short-lived dirtiers
    > (eg. gcc instances in a fast kernel build) may starve long-run dirtiers
    > (eg. dd) as well as pushing the dirty pages to the global hard limit.
    >
    > The solution is to charge the pages dirtied by the exited gcc to the
    > other random gcc/dd instances. It sounds not perfect, however should
    > behave good enough in practice.
    >
    > CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
    > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
    > ---
    > include/linux/writeback.h | 2 ++
    > kernel/exit.c | 2 ++
    > mm/page-writeback.c | 11 +++++++++++
    > 3 files changed, 15 insertions(+)
    >
    > --- linux-next.orig/include/linux/writeback.h 2011-08-08 21:45:58.000000000 +0800
    > +++ linux-next/include/linux/writeback.h 2011-08-08 21:45:58.000000000 +0800
    > @@ -7,6 +7,8 @@
    > #include <linux/sched.h>
    > #include <linux/fs.h>
    >
    > +DECLARE_PER_CPU(int, dirty_leaks);
    > +
    > /*
    > * The 1/4 region under the global dirty thresh is for smooth dirty throttling:
    > *
    > --- linux-next.orig/mm/page-writeback.c 2011-08-08 21:45:58.000000000 +0800
    > +++ linux-next/mm/page-writeback.c 2011-08-08 22:21:50.000000000 +0800
    > @@ -190,6 +190,7 @@ int dirty_ratio_handler(struct ctl_table
    > return ret;
    > }
    >
    > +DEFINE_PER_CPU(int, dirty_leaks) = 0;
    >
    > int dirty_bytes_handler(struct ctl_table *table, int write,
    > void __user *buffer, size_t *lenp,
    > @@ -1150,6 +1151,7 @@ void balance_dirty_pages_ratelimited_nr(
    > {
    > struct backing_dev_info *bdi = mapping->backing_dev_info;
    > int ratelimit;
    > + int *p;
    >
    > if (!bdi_cap_account_dirty(bdi))
    > return;
    > @@ -1158,6 +1160,15 @@ void balance_dirty_pages_ratelimited_nr(
    > if (bdi->dirty_exceeded)
    > ratelimit = 8;
    >
    > + preempt_disable();
    > + p = &__get_cpu_var(dirty_leaks);
    > + if (*p > 0 && current->nr_dirtied < ratelimit) {
    > + nr_pages_dirtied = min(*p, ratelimit - current->nr_dirtied);
    > + *p -= nr_pages_dirtied;
    > + current->nr_dirtied += nr_pages_dirtied;
    > + }
    > + preempt_enable();
    > +

    I think we are still leaking some dirty pages, when the condition is
    false nr_pages_dirtied is just ignored.

    Why not doing something like this?

    current->nr_dirtied += nr_pages_dirtied;
    if (current->nr_dirtied < ratelimit) {
    p = &get_cpu_var(dirty_leaks);
    if (*p > 0) {
    nr_pages_dirtied = min(*p, ratelimit -
    current->nr_dirtied);
    *p -= nr_pages_dirtied;
    } else
    nr_pages_dirtied = 0;
    put_cpu_var(dirty_leaks);

    current->nr_dirtied += nr_pages_dirtied;
    }

    Thanks,
    -Andrea

    > if (unlikely(current->nr_dirtied >= ratelimit))
    > balance_dirty_pages(mapping, current->nr_dirtied);
    > }
    > --- linux-next.orig/kernel/exit.c 2011-08-08 21:43:37.000000000 +0800
    > +++ linux-next/kernel/exit.c 2011-08-08 21:45:58.000000000 +0800
    > @@ -1039,6 +1039,8 @@ NORET_TYPE void do_exit(long code)
    > validate_creds_for_do_exit(tsk);
    >
    > preempt_disable();
    > + if (tsk->nr_dirtied)
    > + __this_cpu_add(dirty_leaks, tsk->nr_dirtied);
    > exit_rcu();
    > /* causes final put_task_struct in finish_task_switch(). */
    > tsk->state = TASK_DEAD;


    \
     
     \ /
      Last update: 2011-08-13 18:31    [W:4.070 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site