Re: [PATCH] writeback: avoid possible balance_dirty_pages() lockup on a light-load bdi

Fengguang Wu
2007-10-02T12:13:27Z

On Mon, Oct 01, 2007 at 07:14:57PM -0700, Andrew Morton wrote:
> On Tue, 2 Oct 2007 10:00:40 +0800 Fengguang Wu <wfg@mail.ustc.edu.cn> wrote:
>
> > writeback: avoid possible balance_dirty_pages() lockup on a light-load bdi
> >
> > On a busy-writing system, a writer could be hold up infinitely on a
> > light-load device. It will be trying to sync more than available dirty data.
> >
> > The problem case:
> >
> > 0. sda/nr_dirty >= dirty_limit;
> > sdb/nr_dirty == 0
> > 1. dd writes 32 pages on sdb
> > 2. balance_dirty_pages() blocks dd, and tries to write 6MB.
> > 3. it never gets there: there's only 128KB dirty data.
> > 4. dd may be blocked for a loooong time
>
> Please quantify loooong.

There're only two 'break' conditions in the loop:
1. nr_dirty + nr_unstable + nr_writeback < dirty_limit
=> *mostly* FALSE for a busy system
=> *always* FALSE in Chakri's stucked NFS case
2. nr_written >= 6MB
for a light-load bdi:
=> *never* TRUE until there comes many new writers, contributing
more dirty pages to sync
=> more worse, those new writers will also stuck here...
the obvious unbalance here is:
each writer contributes only 32KB new dirty pages, but
want to consume (not necessarily available) 6MB

So loooong = min(global-less-busy-time, bdi-many-new-writers-arrival-time).

> > Fix it by returning on 'zero dirty inodes' in the current bdi.
> > (In fact there are slight differences between 'dirty inodes' and 'dirty pages'.
> > But there is no available counters for 'dirty pages'.)
> >
> > But the newly introduced 'break' could make the nr_writeback drift away
> > above the dirty limit. The workaround is to limit the error under 1MB.
>
> I'm still not sure that we fully understand this yet.
>
> If the sdb writer is stuck in balance_dirty_pages() then all sda writers
> will be in balance_dirty_pages() too, madly writing stuff out to sda. And
> pdflush will be writing out sda as well. All this writeout to sda should
> release the sdb writer.
>
> Why isn't this happening?

You are right in the reasoning. The exact consequence is:
the light-load sdb is made as _unresponsive_ as the busy sda

Hence Chakri's case: whenever NFS is stuck, every device get stuck.

>
> > Cc: Chuck Ebbert <cebbert@redhat.com>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
> > ---
> > mm/page-writeback.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > --- linux-2.6.22.orig/mm/page-writeback.c
> > +++ linux-2.6.22/mm/page-writeback.c
> > @@ -250,6 +250,11 @@ static void balance_dirty_pages(struct a
> > pages_written += write_chunk - wbc.nr_to_write;
> > if (pages_written >= write_chunk)
> > break; /* We've done our duty */
> > + if (list_empty(&mapping->host->i_sb->s_dirty) &&
> > + list_empty(&mapping->host->i_sb->s_io) &&
> > + nr_reclaimable + global_page_state(NR_WRITEBACK) <=
> > + dirty_thresh + (1 << (20-PAGE_CACHE_SHIFT)))
> > + break;
> > }
> > congestion_wait(WRITE, HZ/10);
> > }
>
> Well that has a nice safetly net. Perhaps it could fail a bit later on,
> but that depends on why it's failing.

In theory, every CPU/paralle writer could contribute 8 pages of error.
Hence we get 1MB/32KB = 32 (CPUs/writers).

One more serious problem is, a busy writer could also drain all the
dirty pages and make (nr_writeback == dirty_limit+1MB). In that case,
I suspect the light-load sdb writer still have good chance to
make progress(need confirmation).

> How well tested was this?

Not well tested till now. My system becomes unusable soon after
starting the NFS write(even before plugging the network). I'm seeing
large latencies in try_to_wake_up(). Hope that Ingo could help it out.

> If we merge this for 2.6.23 then I expect that we'll immediately unmerge it
> for 2.6.24 because Peter's stuff fixes this problem by other means.
>
> Do we all agree with the above sentence?

Yeah, Peter and me were both aware of the timing.
This patch is only meant for 2.6.23 and 2.6.22.10.

Fengguang