On Thu, Aug 19, 2010 at 12:06:13AM +0800, Wu Fengguang wrote:
> On Wed, Aug 18, 2010 at 04:13:08PM +0200, Johannes Weiner wrote:
> > Hi Matthew,
> >
> > On Tue, Aug 17, 2010 at 03:50:01PM -0400, Matthew Wilcox wrote:
> > >
> > > No comment on this? Was it just that I posted it during the VM summit?
> >
> > I have not forgotten about it. I just have a hard time reproducing
> > those extreme stalls you observed.
> >
> > Running that test on a 2.5GHz machine with 2G of memory gives me
> > stalls of up to half a second. The patchset I am experimenting with
> > gets me down to peaks of 70ms, but it needs further work.
> >
> > Mapped file pages get two rounds on the LRU list, so once the VM
> > starts scanning, it has to go through all of them twice and can only
> > reclaim them on the second encounter.
> >
> > At that point, since we scan without making progress, we start waiting
> > for IO, which is not happening in this case, so we sit there until a
> > timeout expires.
>
> Right, this could lead to some 1s stall. Shaohua and me also noticed
> this when investigating the responsiveness issues. And we are wondering
> if it makes sense to do congestion_wait() only when the bdi is really
> congested? There are no IO underway anyway in this case.
>
> > This stupid-waiting can be improved, and I am working on that. But
>
> Yeah, stupid waiting :)
>
> > since I can not reproduce your observations, I don't know if this is
> > the (sole) source of the problem. Can I send you patches?
>
> Sure.
>
> > > On Mon, Aug 09, 2010 at 09:30:00AM -0400, Matthew Wilcox wrote:
> > > >
> > > > This testcase shows some odd behaviour from the Linux VM.
> > > >
> > > > It creates a 1TB sparse file, mmaps it, and randomly reads locations
> > > > in it. Due to the file being entirely sparse, the VM allocates new pages
> > > > and zeroes them. Initially, it runs very fast, taking on the order of
> > > > 2.7 to 4us per page fault. Eventually, the VM runs out of free pages,
> > > > and starts doing huge amounts of work trying to figure out which of
> > > > these clean pages to throw away.
> >
> > This is similar to one of my test cases for:
> >
> > 6457474 vmscan: detect mapped file pages used only once
> > 31c0569 vmscan: drop page_mapping_inuse()
> > dfc8d63 vmscan: factor out page reference checks
> >
> > because the situation was even worse before (see the series
> > description in dfc8d63). Maybe asking the obvious, but the kernel you
> > tested on did include those commits, right?
> >
> > And just to be sure, I sent you a test-patch to disable the used-once
> > detection on IRC the other day. Did you have time to run it yet?
> > Here it is again:
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 9c7e57c..c757bba 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -584,6 +584,7 @@ static enum page_references page_check_references(struct page *page,
> > return PAGEREF_RECLAIM;
> >
> > if (referenced_ptes) {
> > + return PAGEREF_ACTIVATE;
>
> How come page activation helps?
>
> > if (PageAnon(page))
> > return PAGEREF_ACTIVATE;
> > /*
> >
> >
> > > > In my testing with a 6GB machine and 2.9GHz CPU, one in every
> > > > 15,000 page faults takes over a second, and one in every 40,000
> > > > page faults take over seven seconds!
> > > >
> > > > This test-case demonstrates a problem that occurs with a read-mostly
> > > > mmap of a file on very fast media. I wouldn't like to see a solution
> > > > that special-cases zeroed pages. I think userspace has done its part
> > > > to tell the kernel what's it's doing by calling madvise(MADV_RANDOM).
> > > > This ought to be enough to hint to the kernel that it should be eagerly
> > > > throwing away pages in this VMA.
> >
> > We can probably do something like the following, but I am not sure
> > this is a good fix, either. How many applications are using
> > madvise()?
>
> Heh, it sounds crazy to rip random read pages, though it does help to
> produce a FAST test case.
>
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -495,7 +495,7 @@ int page_referenced_one(struct page *pag
> > * mapping is already gone, the unmap path will have
> > * set PG_referenced or activated the page.
> > */
> > - if (likely(!VM_SequentialReadHint(vma)))
> > + if (likely(!(vma->vm_flags & (VM_SEQ_READ|VM_RAND_READ))))
> > referenced++;
> > }
>
> Thanks,
> Fengguang
>
On Thu, Aug 19, 2010 at 12:07:31AM +0800, Wu, Fengguang wrote:
> On Thu, Aug 19, 2010 at 12:06:13AM +0800, Wu Fengguang wrote:
> > On Wed, Aug 18, 2010 at 04:13:08PM +0200, Johannes Weiner wrote:
> > > Hi Matthew,
> > >
> > > On Tue, Aug 17, 2010 at 03:50:01PM -0400, Matthew Wilcox wrote:
> > > >
> > > > No comment on this? Was it just that I posted it during the VM summit?
> > >
> > > I have not forgotten about it. I just have a hard time reproducing
> > > those extreme stalls you observed.
> > >
> > > Running that test on a 2.5GHz machine with 2G of memory gives me
> > > stalls of up to half a second. The patchset I am experimenting with
> > > gets me down to peaks of 70ms, but it needs further work.
> > >
> > > Mapped file pages get two rounds on the LRU list, so once the VM
> > > starts scanning, it has to go through all of them twice and can only
> > > reclaim them on the second encounter.
> > >
> > > At that point, since we scan without making progress, we start waiting
> > > for IO, which is not happening in this case, so we sit there until a
> > > timeout expires.
> >
> > Right, this could lead to some 1s stall. Shaohua and me also noticed
> > this when investigating the responsiveness issues. And we are wondering
> > if it makes sense to do congestion_wait() only when the bdi is really
> > congested? There are no IO underway anyway in this case.
> >
> > > This stupid-waiting can be improved, and I am working on that. But
> >
> > Yeah, stupid waiting :)
How about this one?
Subject: mm: check device is really congested before sleep in direct page reclaim
congestion_wait() blindly sleep without checking if device is really congested.
In a workload without any write, it can cause direct page reclaim sleep 100ms
and hasn't any help for page reclaim.
There might be other places calling congestion_wait() and need check if
device is really congested, but I can't audit all, so this just changes the
direct page reclaim code path. The new congestion_wait_check() will make sure
at least one device is congested before going into sleep.
Signed-off-by: Shaohua Li <[email protected]>
---
include/linux/backing-dev.h | 1 +
mm/backing-dev.c | 14 ++++++++++++--
mm/vmscan.c | 2 +-
3 files changed, 14 insertions(+), 3 deletions(-)
Index: linux/mm/backing-dev.c
===================================================================
--- linux.orig/mm/backing-dev.c 2010-08-18 16:41:04.000000000 +0800
+++ linux/mm/backing-dev.c 2010-08-19 08:59:14.000000000 +0800
@@ -725,13 +725,16 @@ static wait_queue_head_t congestion_wqh[
__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[1])
};
+static atomic_t nr_congested_bdi[2];
+
void clear_bdi_congested(struct backing_dev_info *bdi, int sync)
{
enum bdi_state bit;
wait_queue_head_t *wqh = &congestion_wqh[sync];
bit = sync ? BDI_sync_congested : BDI_async_congested;
- clear_bit(bit, &bdi->state);
+ if (test_and_clear_bit(bit, &bdi->state))
+ atomic_dec(&nr_congested_bdi[sync]);
smp_mb__after_clear_bit();
if (waitqueue_active(wqh))
wake_up(wqh);
@@ -743,7 +746,8 @@ void set_bdi_congested(struct backing_de
enum bdi_state bit;
bit = sync ? BDI_sync_congested : BDI_async_congested;
- set_bit(bit, &bdi->state);
+ if (!test_and_set_bit(bit, &bdi->state))
+ atomic_inc(&nr_congested_bdi[sync]);
}
EXPORT_SYMBOL(set_bdi_congested);
@@ -769,3 +773,9 @@ long congestion_wait(int sync, long time
}
EXPORT_SYMBOL(congestion_wait);
+long congestion_wait_check(int sync, long timeout)
+{
+ if (atomic_read(&nr_congested_bdi[sync]) == 0)
+ return 0;
+ return congestion_wait(sync, timeout);
+}
Index: linux/include/linux/backing-dev.h
===================================================================
--- linux.orig/include/linux/backing-dev.h 2010-08-18 16:41:04.000000000 +0800
+++ linux/include/linux/backing-dev.h 2010-08-18 16:41:23.000000000 +0800
@@ -285,6 +285,7 @@ enum {
void clear_bdi_congested(struct backing_dev_info *bdi, int sync);
void set_bdi_congested(struct backing_dev_info *bdi, int sync);
long congestion_wait(int sync, long timeout);
+long congestion_wait_check(int sync, long timeout);
static inline bool bdi_cap_writeback_dirty(struct backing_dev_info *bdi)
Index: linux/mm/vmscan.c
===================================================================
--- linux.orig/mm/vmscan.c 2010-08-18 16:41:04.000000000 +0800
+++ linux/mm/vmscan.c 2010-08-18 16:41:23.000000000 +0800
@@ -1910,7 +1910,7 @@ static unsigned long do_try_to_free_page
/* Take a nap, wait for some writeback to complete */
if (!sc->hibernation_mode && sc->nr_scanned &&
priority < DEF_PRIORITY - 2)
- congestion_wait(BLK_RW_ASYNC, HZ/10);
+ congestion_wait_check(BLK_RW_ASYNC, HZ/10);
}
out:
On Thu, Aug 19, 2010 at 12:07:31AM +0800, Wu Fengguang wrote:
> On Thu, Aug 19, 2010 at 12:06:13AM +0800, Wu Fengguang wrote:
> > On Wed, Aug 18, 2010 at 04:13:08PM +0200, Johannes Weiner wrote:
> > > Hi Matthew,
> > >
> > > On Tue, Aug 17, 2010 at 03:50:01PM -0400, Matthew Wilcox wrote:
> > > >
> > > > No comment on this? Was it just that I posted it during the VM summit?
> > >
> > > I have not forgotten about it. I just have a hard time reproducing
> > > those extreme stalls you observed.
> > >
> > > Running that test on a 2.5GHz machine with 2G of memory gives me
> > > stalls of up to half a second. The patchset I am experimenting with
> > > gets me down to peaks of 70ms, but it needs further work.
> > >
> > > Mapped file pages get two rounds on the LRU list, so once the VM
> > > starts scanning, it has to go through all of them twice and can only
> > > reclaim them on the second encounter.
> > >
> > > At that point, since we scan without making progress, we start waiting
> > > for IO, which is not happening in this case, so we sit there until a
> > > timeout expires.
> >
> > Right, this could lead to some 1s stall. Shaohua and me also noticed
> > this when investigating the responsiveness issues. And we are wondering
> > if it makes sense to do congestion_wait() only when the bdi is really
> > congested? There are no IO underway anyway in this case.
I am currently trying to get rid of all the congestion_wait() in the VM.
They are used for different purposes, so they need different replacement
mechanisms.
I saw Shaohua's patch to make congestion_wait() cleverer. But I really
think that congestion is not a good predicate in the first place. Why
would the VM care about IO _congestion_? It needs a bunch of pages to
complete IO, whether the writing device is congested is not really
useful information at this point, I think.
> > > since I can not reproduce your observations, I don't know if this is
> > > the (sole) source of the problem. Can I send you patches?
> >
> > Sure.
Cool!
> > > > On Mon, Aug 09, 2010 at 09:30:00AM -0400, Matthew Wilcox wrote:
> > > > >
> > > > > This testcase shows some odd behaviour from the Linux VM.
> > > > >
> > > > > It creates a 1TB sparse file, mmaps it, and randomly reads locations
> > > > > in it. Due to the file being entirely sparse, the VM allocates new pages
> > > > > and zeroes them. Initially, it runs very fast, taking on the order of
> > > > > 2.7 to 4us per page fault. Eventually, the VM runs out of free pages,
> > > > > and starts doing huge amounts of work trying to figure out which of
> > > > > these clean pages to throw away.
> > >
> > > This is similar to one of my test cases for:
> > >
> > > 6457474 vmscan: detect mapped file pages used only once
> > > 31c0569 vmscan: drop page_mapping_inuse()
> > > dfc8d63 vmscan: factor out page reference checks
> > >
> > > because the situation was even worse before (see the series
> > > description in dfc8d63). Maybe asking the obvious, but the kernel you
> > > tested on did include those commits, right?
> > >
> > > And just to be sure, I sent you a test-patch to disable the used-once
> > > detection on IRC the other day. Did you have time to run it yet?
> > > Here it is again:
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 9c7e57c..c757bba 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -584,6 +584,7 @@ static enum page_references page_check_references(struct page *page,
> > > return PAGEREF_RECLAIM;
> > >
> > > if (referenced_ptes) {
> > > + return PAGEREF_ACTIVATE;
> >
> > How come page activation helps?
This is effectively disabling used-once detection and going back to the old
VM behaviour. I don't think it helps, but this code is recent and directly
related to the test-case. Maybe I/we missed something, it can't hurt to
make sure, right?
On Thu, Aug 19, 2010 at 07:51:06PM +0800, Johannes Weiner wrote:
> I am currently trying to get rid of all the congestion_wait() in the VM.
> They are used for different purposes, so they need different replacement
> mechanisms.
>
> I saw Shaohua's patch to make congestion_wait() cleverer. But I really
> think that congestion is not a good predicate in the first place. Why
> would the VM care about IO _congestion_? It needs a bunch of pages to
> complete IO, whether the writing device is congested is not really
> useful information at this point, I think.
I have the same feeling that the congestion_wait() calls are not
pertinent ones. I'm glad to see people working on that exploring
all possible replacement schemes.
Thanks,
Fengguang
On Thu, Aug 19, 2010 at 07:51:06PM +0800, Johannes Weiner wrote:
> On Thu, Aug 19, 2010 at 12:07:31AM +0800, Wu Fengguang wrote:
> > On Thu, Aug 19, 2010 at 12:06:13AM +0800, Wu Fengguang wrote:
> > > On Wed, Aug 18, 2010 at 04:13:08PM +0200, Johannes Weiner wrote:
> > > > Hi Matthew,
> > > >
> > > > On Tue, Aug 17, 2010 at 03:50:01PM -0400, Matthew Wilcox wrote:
> > > > >
> > > > > No comment on this? Was it just that I posted it during the VM summit?
> > > >
> > > > I have not forgotten about it. I just have a hard time reproducing
> > > > those extreme stalls you observed.
> > > >
> > > > Running that test on a 2.5GHz machine with 2G of memory gives me
> > > > stalls of up to half a second. The patchset I am experimenting with
> > > > gets me down to peaks of 70ms, but it needs further work.
> > > >
> > > > Mapped file pages get two rounds on the LRU list, so once the VM
> > > > starts scanning, it has to go through all of them twice and can only
> > > > reclaim them on the second encounter.
> > > >
> > > > At that point, since we scan without making progress, we start waiting
> > > > for IO, which is not happening in this case, so we sit there until a
> > > > timeout expires.
> > >
> > > Right, this could lead to some 1s stall. Shaohua and me also noticed
> > > this when investigating the responsiveness issues. And we are wondering
> > > if it makes sense to do congestion_wait() only when the bdi is really
> > > congested? There are no IO underway anyway in this case.
>
> I am currently trying to get rid of all the congestion_wait() in the VM.
> They are used for different purposes, so they need different replacement
> mechanisms.
>
> I saw Shaohua's patch to make congestion_wait() cleverer. But I really
> think that congestion is not a good predicate in the first place. Why
> would the VM care about IO _congestion_? It needs a bunch of pages to
> complete IO, whether the writing device is congested is not really
> useful information at this point, I think.
>
> > > > since I can not reproduce your observations, I don't know if this is
> > > > the (sole) source of the problem. Can I send you patches?
> > >
> > > Sure.
>
> Cool!
congestion_wait() isn't the sole source in my test.
with congestion_wait() removed, the max latency is ~50ms.
while if I made the mmaped page reclaimed in one round (makes page_check_references
return PAGEREF_RECLAIM_CLEAN for mmaped pages) in the test, the max latency is ~150us.
Thanks,
Shaohua