Re: [EXPERIMENT] Add new "flush" option

[Andrew Morton <akpm@osdl.org>]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:
>
> Andrew Morton <akpm@osdl.org> writes:
> 
> >> +	if (bdi_write_congested(bdi)) {
> >> +		mod_timer(&sb->flush_timer, jiffies + (HZ / 10));
> >> +		return;
> >> +	}
> >
> > The bdi_write_congested() test probably isn't doing anything useful: it
> > only returns true if there's really heavy writeout in progress.  Possibly
> > you could look at the disk queue accounting stats, work out how much I/O
> > has been happening lately.
> 
> Umm... If queue is too busy, we can't flush immediately. So, this code
> is delaying flush, and we can wait the more user's request by it, I think.

The thing is that bdi_write_congested() will only return true when the
queue is under really really heavy writeout.  Most of the time it just
won't trigger, so this code isn't doing anything very useful.

If you indeed want to implement something like "only sync the device when
it is otherwise idle" then some different approach will be needed.  One
which

a) detects when there is light writeout happening and which

b) detects when there are reads happening too.

I don't know whether any of this is particularly useful, really.  So I'd
suggest that we just remove the bdi_write_contested() test and leave it at
that.

I could be wrong, of course - if you have some testcase in which the
bdi_write_congested() test makes some perceptible difference then I'd be
interested in hearing about it.  If you put a printk in there, does it
trigger much?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/