All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Rik van Riel <riel@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: PERF: performance tests with the split LRU VM in -mm
Date: Mon, 28 Jul 2008 17:17:28 -0700	[thread overview]
Message-ID: <20080728171728.7d0452bc.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080728200311.2218af4e@cuia.bos.redhat.com>

On Mon, 28 Jul 2008 20:03:11 -0400
Rik van Riel <riel@redhat.com> wrote:

> On Mon, 28 Jul 2008 19:57:13 -0400
> Rik van Riel <riel@redhat.com> wrote:
> > On Mon, 28 Jul 2008 16:41:24 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > > Andrew, what is your preference between:
> > > > 	http://lkml.org/lkml/2008/7/15/465
> > > > and
> > > > 	http://marc.info/?l=linux-mm&m=121683855132630&w=2
> > > > 
> > > 
> > > Boy.  They both seem rather hacky special-cases.  But that doesn't mean
> > > that they're undesirable hacky special-cases.  I guess the second one
> > > looks a bit more "algorithmic" and a bit less hacky-special-case.  But
> > > it all depends on testing..
> > 
> > I prefer the second one, since it removes the + 1 magic (at least,
> > for the higher priorities), instead of adding new magic like the
> > other patch does.
> 
> Btw, didn't you add that "+ 1" originally early on in the 2.6 VM?

You mean this?

		/*
		 * Add one to nr_to_scan just to make sure that the kernel
		 * will slowly sift through the active list.
		 */
		zone->nr_scan_active +=
			(zone_page_state(zone, NR_ACTIVE) >> priority) + 1;


> Do you remember its purpose?  

erm, not specifically, but I tended to lavishly describe changes like
this in the changelogging.

> Does it still make sense to have that "+ 1" in the split LRU VM?
> 
> Could we get away with just removing it unconditionally?

We should do the necessary git dumpster-diving before tossing out
hard-won changes.  Otherwise we might need to spend a year
re-discovering and re-fixing already-discovered-and-fixed things.

That code has been there in one way or another for some time.

In June 2004, 385c0449 did this:

        /*
-        * Try to keep the active list 2/3 of the size of the cache.  And
-        * make sure that refill_inactive is given a decent number of pages.
-        *
-        * The "scan_active + 1" here is important.  With pagecache-intensive
-        * workloads the inactive list is huge, and `ratio' evaluates to zero
-        * all the time.  Which pins the active list memory.  So we add one to
-        * `scan_active' just to make sure that the kernel will slowly sift
-        * through the active list.
+        * Add one to `nr_to_scan' just to make sure that the kernel will
+        * slowly sift through the active list.
         */
-       if (zone->nr_active >= 4*(zone->nr_inactive*2 + 1)) {
-               /* Don't scan more than 4 times the inactive list scan size */
-               scan_active = 4*scan_inactive;

(there was some regrettable information loss there).

Is the scenario which that fix addresses no longer possible?


On a different topic, I am staring in frustration at
introduce-__get_user_pages.patch, which says:

  New munlock processing need to GUP_FLAGS_IGNORE_VMA_PERMISSIONS. 
  because current get_user_pages() can't grab PROT_NONE pages theresore
  it cause PROT_NONE pages can't munlock.

could someone please work out for me which of these patches:

vmscan-move-isolate_lru_page-to-vmscanc.patch
vmscan-use-an-indexed-array-for-lru-variables.patch
swap-use-an-array-for-the-lru-pagevecs.patch
vmscan-free-swap-space-on-swap-in-activation.patch
define-page_file_cache-function.patch
vmscan-split-lru-lists-into-anon-file-sets.patch
vmscan-second-chance-replacement-for-anonymous-pages.patch
vmscan-fix-pagecache-reclaim-referenced-bit-check.patch
vmscan-add-newly-swapped-in-pages-to-the-inactive-list.patch
more-aggressively-use-lumpy-reclaim.patch
pageflag-helpers-for-configed-out-flags.patch
unevictable-lru-infrastructure.patch
unevictable-lru-page-statistics.patch
ramfs-and-ram-disk-pages-are-unevictable.patch
shm_locked-pages-are-unevictable.patch
mlock-mlocked-pages-are-unevictable.patch
mlock-downgrade-mmap-sem-while-populating-mlocked-regions.patch
mmap-handle-mlocked-pages-during-map-remap-unmap.patch

that patch fixes?

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Rik van Riel <riel@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: PERF: performance tests with the split LRU VM in -mm
Date: Mon, 28 Jul 2008 17:17:28 -0700	[thread overview]
Message-ID: <20080728171728.7d0452bc.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080728200311.2218af4e@cuia.bos.redhat.com>

On Mon, 28 Jul 2008 20:03:11 -0400
Rik van Riel <riel@redhat.com> wrote:

> On Mon, 28 Jul 2008 19:57:13 -0400
> Rik van Riel <riel@redhat.com> wrote:
> > On Mon, 28 Jul 2008 16:41:24 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > > Andrew, what is your preference between:
> > > > 	http://lkml.org/lkml/2008/7/15/465
> > > > and
> > > > 	http://marc.info/?l=linux-mm&m=121683855132630&w=2
> > > > 
> > > 
> > > Boy.  They both seem rather hacky special-cases.  But that doesn't mean
> > > that they're undesirable hacky special-cases.  I guess the second one
> > > looks a bit more "algorithmic" and a bit less hacky-special-case.  But
> > > it all depends on testing..
> > 
> > I prefer the second one, since it removes the + 1 magic (at least,
> > for the higher priorities), instead of adding new magic like the
> > other patch does.
> 
> Btw, didn't you add that "+ 1" originally early on in the 2.6 VM?

You mean this?

		/*
		 * Add one to nr_to_scan just to make sure that the kernel
		 * will slowly sift through the active list.
		 */
		zone->nr_scan_active +=
			(zone_page_state(zone, NR_ACTIVE) >> priority) + 1;


> Do you remember its purpose?  

erm, not specifically, but I tended to lavishly describe changes like
this in the changelogging.

> Does it still make sense to have that "+ 1" in the split LRU VM?
> 
> Could we get away with just removing it unconditionally?

We should do the necessary git dumpster-diving before tossing out
hard-won changes.  Otherwise we might need to spend a year
re-discovering and re-fixing already-discovered-and-fixed things.

That code has been there in one way or another for some time.

In June 2004, 385c0449 did this:

        /*
-        * Try to keep the active list 2/3 of the size of the cache.  And
-        * make sure that refill_inactive is given a decent number of pages.
-        *
-        * The "scan_active + 1" here is important.  With pagecache-intensive
-        * workloads the inactive list is huge, and `ratio' evaluates to zero
-        * all the time.  Which pins the active list memory.  So we add one to
-        * `scan_active' just to make sure that the kernel will slowly sift
-        * through the active list.
+        * Add one to `nr_to_scan' just to make sure that the kernel will
+        * slowly sift through the active list.
         */
-       if (zone->nr_active >= 4*(zone->nr_inactive*2 + 1)) {
-               /* Don't scan more than 4 times the inactive list scan size */
-               scan_active = 4*scan_inactive;

(there was some regrettable information loss there).

Is the scenario which that fix addresses no longer possible?


On a different topic, I am staring in frustration at
introduce-__get_user_pages.patch, which says:

  New munlock processing need to GUP_FLAGS_IGNORE_VMA_PERMISSIONS. 
  because current get_user_pages() can't grab PROT_NONE pages theresore
  it cause PROT_NONE pages can't munlock.

could someone please work out for me which of these patches:

vmscan-move-isolate_lru_page-to-vmscanc.patch
vmscan-use-an-indexed-array-for-lru-variables.patch
swap-use-an-array-for-the-lru-pagevecs.patch
vmscan-free-swap-space-on-swap-in-activation.patch
define-page_file_cache-function.patch
vmscan-split-lru-lists-into-anon-file-sets.patch
vmscan-second-chance-replacement-for-anonymous-pages.patch
vmscan-fix-pagecache-reclaim-referenced-bit-check.patch
vmscan-add-newly-swapped-in-pages-to-the-inactive-list.patch
more-aggressively-use-lumpy-reclaim.patch
pageflag-helpers-for-configed-out-flags.patch
unevictable-lru-infrastructure.patch
unevictable-lru-page-statistics.patch
ramfs-and-ram-disk-pages-are-unevictable.patch
shm_locked-pages-are-unevictable.patch
mlock-mlocked-pages-are-unevictable.patch
mlock-downgrade-mmap-sem-while-populating-mlocked-regions.patch
mmap-handle-mlocked-pages-during-map-remap-unmap.patch

that patch fixes?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2008-07-29  0:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-25  2:25 PERF: performance tests with the split LRU VM in -mm Rik van Riel
2008-07-25  2:25 ` Rik van Riel
2008-07-28 14:57 ` Rik van Riel
2008-07-28 14:57   ` Rik van Riel
2008-07-28 15:30   ` Ray Lee
2008-07-28 15:30     ` Ray Lee
2008-07-28 23:41   ` Andrew Morton
2008-07-28 23:41     ` Andrew Morton
2008-07-28 23:57     ` Rik van Riel
2008-07-28 23:57       ` Rik van Riel
2008-07-29  0:03       ` Rik van Riel
2008-07-29  0:03         ` Rik van Riel
2008-07-29  0:17         ` Andrew Morton [this message]
2008-07-29  0:17           ` Andrew Morton
2008-07-29  0:31           ` Rik van Riel
2008-07-29  0:31             ` Rik van Riel
2008-07-29  0:46           ` Lee Schermerhorn
2008-07-29  0:46             ` Lee Schermerhorn
2008-07-29 13:21         ` Johannes Weiner
2008-07-29 13:21           ` Johannes Weiner
2008-07-29 13:28           ` Rik van Riel
2008-07-29 13:28             ` Rik van Riel
2008-07-29 13:04 ` KOSAKI Motohiro
2008-07-29 13:04   ` KOSAKI Motohiro
2008-07-29 13:16   ` Rik van Riel
2008-07-29 13:16     ` Rik van Riel
2008-07-29 13:51 ` Johannes Weiner
2008-07-29 13:51   ` Johannes Weiner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080728171728.7d0452bc.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.