linux-kernel - Re: [PATCH 14/18] writeback: control dirty pause time

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 7 Sep 2011 10:02:14 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kara <jack@...e.cz>, Christoph Hellwig <hch@....de>,
	Dave Chinner <david@...morbit.com>,
	Greg Thelen <gthelen@...gle.com>,
	Minchan Kim <minchan.kim@...il.com>,
	Vivek Goyal <vgoyal@...hat.com>,
	Andrea Righi <arighi@...eler.com>,
	linux-mm <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 14/18] writeback: control dirty pause time

On Tue, Sep 06, 2011 at 11:51:25PM +0800, Peter Zijlstra wrote:
> On Sun, 2011-09-04 at 09:53 +0800, Wu Fengguang wrote:
> > plain text document attachment (max-pause-adaption)
> > The dirty pause time shall ultimately be controlled by adjusting
> > nr_dirtied_pause, since there is relationship
> > 
> > 	pause = pages_dirtied / task_ratelimit
> > 
> > Assuming
> > 
> > 	pages_dirtied ~= nr_dirtied_pause
> > 	task_ratelimit ~= dirty_ratelimit
> > 
> > We get
> > 
> > 	nr_dirtied_pause ~= dirty_ratelimit * desired_pause
> > 
> > Here dirty_ratelimit is preferred over task_ratelimit because it's
> > more stable.
> > 
> > It's also important to limit possible large transitional errors:
> > 
> > - bw is changing quickly
> > - pages_dirtied << nr_dirtied_pause on entering dirty exceeded area
> > - pages_dirtied >> nr_dirtied_pause on btrfs (to be improved by a
> >   separate fix, but still expect non-trivial errors)
> > 
> > So we end up using the above formula inside clamp_val().
> > 
> > The best test case for this code is to run 100 "dd bs=4M" tasks on
> > btrfs and check its pause time distribution.
> 
> 
> 
> > Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> > ---
> >  mm/page-writeback.c |   15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > --- linux-next.orig/mm/page-writeback.c	2011-08-29 19:08:43.000000000 +0800
> > +++ linux-next/mm/page-writeback.c	2011-08-29 19:08:44.000000000 +0800
> > @@ -1193,7 +1193,20 @@ pause:
> >  	if (!dirty_exceeded && bdi->dirty_exceeded)
> >  		bdi->dirty_exceeded = 0;
> >  
> > -	current->nr_dirtied_pause = dirty_poll_interval(nr_dirty, dirty_thresh);
> > +	if (pause == 0)
> > +		current->nr_dirtied_pause =
> > +				dirty_poll_interval(nr_dirty, dirty_thresh);
> > +	else if (period <= max_pause / 4 &&
> > +		 pages_dirtied >= current->nr_dirtied_pause)
> > +		current->nr_dirtied_pause = clamp_val(
> > +					dirty_ratelimit * (max_pause / 2) / HZ,
> > +					pages_dirtied + pages_dirtied / 8,
> > +					pages_dirtied * 4);
> > +	else if (pause >= max_pause)
> > +		current->nr_dirtied_pause = 1 | clamp_val(
> > +					dirty_ratelimit * (max_pause * 3/8)/HZ,
> > +					pages_dirtied / 4,
> > +					pages_dirtied * 7/8);
> >  
> 
> I very much prefer { } over multi line stmts, even if not strictly
> needed.

Yeah, that does look better.

> I'm also not quite sure why pause==0 is a special case,

Good question, it covers the important case that dirty pages are still
in the freerun area, where we don't do pause at all and hence cannot
adaptively adjust current->nr_dirtied_pause based on the pause time.

I'll add a simple comment for that condition:

        if (pause == 0) { /* in freerun area */

> also, do the two other line segments connect on the transition
> point?

I guess we can simply unify the other two formulas into one:

        } else if (period <= max_pause / 4 &&
                 pages_dirtied >= current->nr_dirtied_pause) {
                current->nr_dirtied_pause = clamp_val(
==>                                     dirty_ratelimit * (max_pause / 2) / HZ,
                                        pages_dirtied + pages_dirtied / 8,
                                        pages_dirtied * 4);
        } else if (pause >= max_pause) {
                current->nr_dirtied_pause = 1 | clamp_val(
==>                                     dirty_ratelimit * (max_pause / 2) / HZ,
                                        pages_dirtied / 4,
                                        pages_dirtied - pages_dirtied / 8);
        }

Thanks,
Fengguang
--
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/