From: Johannes Weiner <hannes@cmpxchg.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Vladimir Davydov <vdavydov@virtuozzo.com>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: vmpressure: fix scan window after SWAP_CLUSTER_MAX increase
Date: Mon, 16 Nov 2015 15:22:54 -0500 [thread overview]
Message-ID: <20151116202254.GA6996@cmpxchg.org> (raw)
In-Reply-To: <alpine.LSU.2.11.1510211303240.3467@eggly.anvils>
Dear Hugh,
[ sorry, I just noticed this email now ]
On Wed, Oct 21, 2015 at 01:05:53PM -0700, Hugh Dickins wrote:
> On Wed, 21 Oct 2015, Johannes Weiner wrote:
> > On Mon, Oct 19, 2015 at 02:13:01PM -0400, Johannes Weiner wrote:
> > > mm-increase-swap_cluster_max-to-batch-tlb-flushes.patch changed
> > > SWAP_CLUSTER_MAX from 32 pages to 256 pages, inadvertantly switching
> > > the scan window for vmpressure detection from 2MB to 16MB. Revert.
> > >
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > ---
> > > mm/vmpressure.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> > > index c5afd57..74f206b 100644
> > > --- a/mm/vmpressure.c
> > > +++ b/mm/vmpressure.c
> > > @@ -38,7 +38,7 @@
> > > * TODO: Make the window size depend on machine size, as we do for vmstat
> > > * thresholds. Currently we set it to 512 pages (2MB for 4KB pages).
> > > */
> > > -static const unsigned long vmpressure_win = SWAP_CLUSTER_MAX * 16;
> > > +static const unsigned long vmpressure_win = SWAP_CLUSTER_MAX;
> >
> > Argh, Mel's patch sets SWAP_CLUSTER_MAX to 256, so this should be
> > SWAP_CLUSTER_MAX * 2 to retain the 512 pages scan window.
> >
> > Andrew could you please update this fix in-place? Otherwise I'll
> > resend a corrected version.
> >
> > Thanks, and sorry about that.
>
> I don't understand why "SWAP_CLUSTER_MAX * 2" is thought better than "512".
> Retaining a level of obscurity, that just bit us twice, is a good thing?
I'm not sure it is. But it doesn't seem entirely wrong to link it to
the reclaim scan window, either--at least be a multiple of it so that
the vmpressure reporting happens cleanly at the end of a scan cycle?
I don't mind changing it to 512, but it doesn't feel like an obvious
improvement, either.
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Vladimir Davydov <vdavydov@virtuozzo.com>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: vmpressure: fix scan window after SWAP_CLUSTER_MAX increase
Date: Mon, 16 Nov 2015 15:22:54 -0500 [thread overview]
Message-ID: <20151116202254.GA6996@cmpxchg.org> (raw)
In-Reply-To: <alpine.LSU.2.11.1510211303240.3467@eggly.anvils>
Dear Hugh,
[ sorry, I just noticed this email now ]
On Wed, Oct 21, 2015 at 01:05:53PM -0700, Hugh Dickins wrote:
> On Wed, 21 Oct 2015, Johannes Weiner wrote:
> > On Mon, Oct 19, 2015 at 02:13:01PM -0400, Johannes Weiner wrote:
> > > mm-increase-swap_cluster_max-to-batch-tlb-flushes.patch changed
> > > SWAP_CLUSTER_MAX from 32 pages to 256 pages, inadvertantly switching
> > > the scan window for vmpressure detection from 2MB to 16MB. Revert.
> > >
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > ---
> > > mm/vmpressure.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> > > index c5afd57..74f206b 100644
> > > --- a/mm/vmpressure.c
> > > +++ b/mm/vmpressure.c
> > > @@ -38,7 +38,7 @@
> > > * TODO: Make the window size depend on machine size, as we do for vmstat
> > > * thresholds. Currently we set it to 512 pages (2MB for 4KB pages).
> > > */
> > > -static const unsigned long vmpressure_win = SWAP_CLUSTER_MAX * 16;
> > > +static const unsigned long vmpressure_win = SWAP_CLUSTER_MAX;
> >
> > Argh, Mel's patch sets SWAP_CLUSTER_MAX to 256, so this should be
> > SWAP_CLUSTER_MAX * 2 to retain the 512 pages scan window.
> >
> > Andrew could you please update this fix in-place? Otherwise I'll
> > resend a corrected version.
> >
> > Thanks, and sorry about that.
>
> I don't understand why "SWAP_CLUSTER_MAX * 2" is thought better than "512".
> Retaining a level of obscurity, that just bit us twice, is a good thing?
I'm not sure it is. But it doesn't seem entirely wrong to link it to
the reclaim scan window, either--at least be a multiple of it so that
the vmpressure reporting happens cleanly at the end of a scan cycle?
I don't mind changing it to 512, but it doesn't feel like an obvious
improvement, either.
next prev parent reply other threads:[~2015-11-16 20:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-19 18:13 [PATCH] mm: vmpressure: fix scan window after SWAP_CLUSTER_MAX increase Johannes Weiner
2015-10-19 18:13 ` Johannes Weiner
2015-10-19 18:13 ` Johannes Weiner
2015-10-20 7:47 ` Mel Gorman
2015-10-20 7:47 ` Mel Gorman
2015-10-20 13:50 ` Johannes Weiner
2015-10-20 13:50 ` Johannes Weiner
[not found] ` <20151020135056.GA22383-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2015-10-21 9:17 ` Mel Gorman
2015-10-21 9:17 ` Mel Gorman
2015-10-21 9:17 ` Mel Gorman
[not found] ` <1445278381-21033-1-git-send-email-hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2015-10-21 19:38 ` Johannes Weiner
2015-10-21 19:38 ` Johannes Weiner
2015-10-21 19:38 ` Johannes Weiner
2015-10-21 20:05 ` Hugh Dickins
2015-10-21 20:05 ` Hugh Dickins
2015-11-16 20:22 ` Johannes Weiner [this message]
2015-11-16 20:22 ` Johannes Weiner
2015-12-02 10:11 ` Hugh Dickins
2015-12-02 10:11 ` Hugh Dickins
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=20151116202254.GA6996@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=vdavydov@virtuozzo.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.