linux-kernel - Re: [PATCH 4/7] writeback: introduce max-pause and pass-good dirty limits

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <20110621172043.51621aa9.akpm@linux-foundation.org>
Date:	Tue, 21 Jun 2011 17:20:43 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Wu Fengguang <fengguang.wu@...el.com>
Cc:	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	Jan Kara <jack@...e.cz>, Dave Chinner <david@...morbit.com>,
	Christoph Hellwig <hch@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/7] writeback: introduce max-pause and pass-good dirty
 limits

On Sun, 19 Jun 2011 23:01:12 +0800
Wu Fengguang <fengguang.wu@...el.com> wrote:

> The max-pause limit helps to keep the sleep time inside
> balance_dirty_pages() within 200ms. The 200ms max sleep means per task
> rate limit of 8pages/200ms=160KB/s, which normally is enough to stop
> dirtiers from continue pushing the dirty pages high, unless there are
> a sufficient large number of slow dirtiers (ie. 500 tasks doing 160KB/s
> will still sum up to 80MB/s, reaching the write bandwidth of a slow disk).
> 
> The pass-good limit helps to let go of the good bdi's in the presence of
> a blocked bdi (ie. NFS server not responding) or slow USB disk which for
> some reason build up a large number of initial dirty pages that refuse
> to go away anytime soon.

The hard-wired numbers and hard-wired assumptions about device speeds
shouldn't be here at all.  They will be sub-optimal (and sometimes
extremely so) for all cases.  They will become wronger over time.  Or
less wrong, depending upon which way they were originally wrong.

> +		dirty_thresh = hard_dirty_limit(dirty_thresh);
> +		if (nr_dirty < dirty_thresh + dirty_thresh / DIRTY_MAXPAUSE &&
> +		    jiffies - start_time > MAX_PAUSE)
> +			break;
> +		if (nr_dirty < dirty_thresh + dirty_thresh / DIRTY_PASSGOOD &&
> +		    bdi_dirty < bdi_thresh)
> +			break;

It appears that despite their similarity, DIRTY_MAXPAUSE is a
dimensionless value whereas the units of MAX_PAUSE is jiffies.  Perhaps
more care in naming choices would clarify things like this.

The first comparison might be clearer if it used time_after().

Both statements need comments explaining what they do and *why they do
it*.

--
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/