lkml.org 
[lkml]   [2010]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH 0/3] Avoid the use of congestion_wait under zone pressure
On Fri, Mar 12, 2010 at 02:05:26AM -0500, Andrew Morton wrote:
> On Fri, 12 Mar 2010 07:39:26 +0100 Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com> wrote:
>
> >
> >
> > Andrew Morton wrote:
> > > On Mon, 8 Mar 2010 11:48:20 +0000
> > > Mel Gorman <mel@csn.ul.ie> wrote:
> > >
> > >> Under memory pressure, the page allocator and kswapd can go to sleep using
> > >> congestion_wait(). In two of these cases, it may not be the appropriate
> > >> action as congestion may not be the problem.
> > >
> > > clear_bdi_congested() is called each time a write completes and the
> > > queue is below the congestion threshold.
> > >
> > > So if the page allocator or kswapd call congestion_wait() against a
> > > non-congested queue, they'll wake up on the very next write completion.
> >
> > Well the issue came up in all kind of loads where you don't have any
> > writes at all that can wake up congestion_wait.
> > Thats true for several benchmarks, but also real workload as well e.g. A
> > backup job reading almost all files sequentially and pumping out stuff
> > via network.
>
> Why is reclaim going into congestion_wait() at all if there's heaps of
> clean reclaimable pagecache lying around?
>
> (I don't thing the read side of the congestion_wqh[] has ever been used, btw)
>

I believe it's a race albeit one that has been there a long time.

In __alloc_pages_direct_reclaim, a process does approximately the
following

1. Enters direct reclaim
2. Calls cond_reched()
3. Drain pages if necessary
4. Attempt to allocate a page

Between steps 2 and 3, it's possible to have reclaimed the pages but
another process allocate them. It then proceeds and decides try again
but calls congestion_wait() before it loops around.

Plenty of read cache reclaimed but no forward progress.

> > > Hence the above-quoted claim seems to me to be a significant mis-analysis and
> > > perhaps explains why the patchset didn't seem to help anything?
> >
> > While I might have misunderstood you and it is a mis-analysis in your
> > opinion, it fixes a -80% Throughput regression on sequential read
> > workloads, thats not nothing - its more like absolutely required :-)
> >
> > You might check out the discussion with the subject "Performance
> > regression in scsi sequential throughput (iozone) due to "e084b -
> > page-allocator: preserve PFN ordering when __GFP_COLD is set"".
> > While the original subject is misleading from todays point of view, it
> > contains a lengthy discussion about exactly when/why/where time is lost
> > due to congestion wait with a lot of traces, counters, data attachments
> > and such stuff.
>
> Well if we're not encountering lots of dirty pages in reclaim then we
> shouldn't be waiting for writes to retire, of course.
>
> But if we're not encountering lots of dirty pages in reclaim, we should
> be reclaiming pages, normally.
>

We probably are.

> I could understand reclaim accidentally going into congestion_wait() if
> it hit a large pile of pages which are unreclaimable for reasons other
> than being dirty, but is that happening in this case?
>

Probably not. It's almost certainly the race I described above.

> If not, we broke it again.
>

We were broken with respect to this in the first place. That
cond_reched() is badly placed and waiting on congestion when congestion
might not be involved is also a bit odd.

It's possible that Christian's specific problem would also be addressed
by the following patch. Christian, willing to test?

It still feels a bit unnatural though that the page allocator waits on
congestion when what it really cares about is watermarks. Even if this
patch works for Christian, I think it still has merit so will kick it a
few more times.

==== CUT HERE ====
page-allocator: Attempt page allocation immediately after direct reclaim

After a process completes direct reclaim it calls cond_resched() as
potentially it has been running a long time. When it wakes up, it
attempts to allocate a page. There is a large window during which
another process can allocate the pages reclaimed by direct reclaim. This
patch attempts to allocate a page immediately after direct reclaim but
will still go to sleep afterwards if its quantum has expired.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---
mm/page_alloc.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a8182c8..973b7fc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1721,8 +1721,6 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
lockdep_clear_current_reclaim_state();
p->flags &= ~PF_MEMALLOC;

- cond_resched();
-
if (order != 0)
drain_all_pages();

@@ -1731,6 +1729,9 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
zonelist, high_zoneidx,
alloc_flags, preferred_zone,
migratetype);
+
+ cond_resched();
+
return page;
}


\
 
 \ /
  Last update: 2010-03-12 11:49    [W:0.086 / U:7.820 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site