linux-kernel - Re: [PATCH 06/13] writeback: bdi write bandwidth estimation

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]

Date:	Wed, 24 Nov 2010 20:10:46 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, Jan Kara <jack@...e.cz>,
	"Li, Shaohua" <shaohua.li@...el.com>,
	Christoph Hellwig <hch@....de>,
	Dave Chinner <david@...morbit.com>,
	Theodore Ts'o <tytso@....edu>,
	Chris Mason <chris.mason@...cle.com>,
	Mel Gorman <mel@....ul.ie>, Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	linux-mm <linux-mm@...ck.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 06/13] writeback: bdi write bandwidth estimation

On Wed, Nov 24, 2010 at 07:05:32PM +0800, Peter Zijlstra wrote:
> On Wed, 2010-11-17 at 12:27 +0800, Wu Fengguang wrote:
> > +void bdi_update_write_bandwidth(struct backing_dev_info *bdi,
> > +                               unsigned long *bw_time,
> > +                               s64 *bw_written)
> > +{
> > +       unsigned long written;
> > +       unsigned long elapsed;
> > +       unsigned long bw;
> > +       unsigned long w;
> > +
> > +       if (*bw_written == 0)
> > +               goto snapshot;
> > +
> > +       elapsed = jiffies - *bw_time;
> > +       if (elapsed < HZ/100)
> > +               return;
> > +
> > +       /*
> > +        * When there lots of tasks throttled in balance_dirty_pages(), they
> > +        * will each try to update the bandwidth for the same period, making
> > +        * the bandwidth drift much faster than the desired rate (as in the
> > +        * single dirtier case). So do some rate limiting.
> > +        */
> > +       if (jiffies - bdi->write_bandwidth_update_time < elapsed)
> > +               goto snapshot;
>
> Why this goto snapshot and not simply return? This is the second call
> (bdi_update_bandwidth equivalent).

Good question. The loop inside balance_dirty_pages() normally run only
once, however wb_writeback() may loop over and over again. If we just
return here, the condition

        (jiffies - bdi->write_bandwidth_update_time < elapsed)

cannot be reset, then future bdi_update_bandwidth() calls in the same
wb_writeback() loop will never find it OK to update the bandwidth.

It does assume no races between CPUs.. We may need some per-cpu based
estimation.

> If you were to leave the old bw_written/bw_time in place the next loop
> around in wb_writeback() would see a larger delta..
>
> I guess this funny loop in wb_writeback() is also the reason you've got
> a single function and not the get/update like separation

I do the single function mainly for wb_writeback(), where it
continuously updates bandwidth inside the loop. The function can be
called in such way:

loop {
        take snapshot on first call

        no action if recalled within 10ms
        ...
        no action if recalled within 10ms
        ...

        update bandwidth and prepare for next update by taking the snapshot

        no action if recalled within 10ms
        ...
}

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/