lkml.org 
[lkml]   [2010]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 31/45] writeback: sync old inodes first in background writeback
On Mon, Jul 12, 2010 at 11:01:29AM +0800, Christoph Hellwig wrote:
> On Wed, Oct 07, 2009 at 03:38:49PM +0800, Wu Fengguang wrote:
> > A background flush work may run for ever. So it's reasonable for it to
> > mimic the kupdate behavior of syncing old/expired inodes first.
>
> I've looked at this a bit again after you pointed to this thread in
> the direct reclaim thread, and I think we should be even more aggressive
> in pushing out old inodes.

Agreed.

> We basically have two types of I/O done from wb_do_writeback:
>
> - either we want to write all inodes for a given bdi/superblock. That
> includes all WB_SYNC_ALL callers, but also things like
> writeback_inodes_sb and the wakeup_flusher_threads call from
> sys_sync.
> - or we have a specific goal, like for the background writeback or
> the wakeup_flusher_threads from free_more_memory.
>
> For the first case there's obviously no point in doing any
> older_than_this processing as we write out all inodes anyway.

We may also do older_than_this even for the sync-the-whole-world case,
as long as this simplifies wb_writeback() and/or other code. This may
make a difference for slow devices.

> For the second case we should always do a older_than_this pass _first_.

Agree in general.

> Rationale: we really should get the old inodes out ASAP, so that we
> keep the amount of changes lost on a crash in the boundaries.
> Furthermore the callers only need N pages cleaned, and they don't care
> where from. So if we can reach our goal with the older_than_this
> writeback we're fine. If the writeback loop is long enough we can
> keep doing more of these later on as well.

Right.

> Doing this should also help cleaning the code up a bit by moving the
> wb_check_old_data_flush logic into wb_writeback and getting rid of the
> for_kupdate paramter in struct wb_writeback_work. I'm not even sure
> it's worth keeping it in struct writeback_control.

I'd also like to see less for_kupdate tests. Whether or not we can
totally get rid of the explicit for_kupdate case, there are always
four main writeback goals/semantics:
- periodic stop when all 30s-old inodes are written
- background stop when background threshold is reached
- nr_pages stop when nr_pages written (or when all clean)
- sync stop when all older-than-sync-time inodes are written

Note that
- the "sync" goal is obviously a superset of the "periodic" goal
- the "background" goal may be expanded to include the "periodic" goal
- the latter three goals may all do some "periodic" goal loops, with
a moving "old" criterion.

Thanks,
Fengguang


\
 
 \ /
  Last update: 2010-07-12 17:27    [W:0.340 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site