Re: [PATCH] mm: vmpressure: fix scan window after SWAP_CLUSTER_MAX increase

[Johannes Weiner]

On Tue, Oct 20, 2015 at 08:47:00AM +0100, Mel Gorman 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@xxxxxxxxxxx>
> 
> This was known at the time but it was not clear what the measurable
> impact would be. VM Pressure is odd in that it gives strange results at
> times anyway, particularly on NUMA machines.

Interesting. Strange how? In terms of reporting random flukes?

I'm interested in vmpressure because I think reclaim efficiency would
be a useful metric to export to other parts of the kernel. We have
slab shrinkers that export LRU scan rate for ageable caches--the
keyword being "ageable" here--that are currently used for everything
that wants to know about memory pressure. But unless you actually age
and rotate your objects, it does not make sense to shrink your objects
based on LRU scan rate. There is no reason to drop your costly objects
if all the VM does is pick up streaming cache pages. In those cases,
it would make more sense to hook yourself up to be notified when
reclaim efficiency drops below a certain threshold.

> To be honest, it still isn't clear to me what the impact of the
> patch is. With different base page sizes (e.g. on ppc64 with some
> configs), the window is still large. At the time, it was left as-is
> as I could not decide one way or the other but I'm ok with restoring
> the behaviour so either way;
> 
> Acked-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>

Thanks!

> Out of curiosity though, what *is* the user-visible impact of the patch
> though? It's different but I'm having trouble deciding if it's better
> or worse. I'm curious as to whether the patch is based on a bug report
> or intuition.

No, I didn't observe a bug, it just struck me during code review.

My sole line of reasoning was: the pressure scan window was chosen
back then to be at a certain point between reliable sample size and
on-time reporting of detected pressure. Increasing the LRU scan window
for the purpose of improved TLB batching seems sufficiently unrelated
that we wouldn't want to change the vmpressure window as a side effect.

Arguably it should not even reference SWAP_CLUSTER_MAX, but on the
other hand that value has been pretty static in the past, and it looks
better than '256' :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/