lkml.org 
[lkml]   [2012]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [ 101/175] mm: vmscan: forcibly scan highmem if there are too many buffer_heads pinning highmem
On Fri, 6 Apr 2012, Nikola Ciprich wrote:
> Hi, sorry it took me a bit longer.
>
> here's my backport,

To 3.0.27 I presume; I've not tried it against 3.2.14,
haven't checked if that would be much the same or not.

> compiles fine, kernel boots without any problems.
> please review.

Thank you for doing the work, but I'm afraid it looks wrong to me.
I'd be more confident to leave it to Mel myself. Comments below.

> n.
>
> Signed-off-by: Nikola Ciprich <nikola.ciprich@linuxbox.cz
>
> (backport of upstream commit cc715d99e529d470dde2f33a6614f255adea71f3)
>
> mm: vmscan: forcibly scan highmem if there are too many buffer_heads pinning highmem
>
> Stuart Foster reported on bugzilla that copying large amounts of data
> from NTFS caused an OOM kill on 32-bit X86 with 16G of memory. Andrew
> Morton correctly identified that the problem was NTFS was using 512
> blocks meaning each page had 8 buffer_heads in low memory pinning it.
>
> In the past, direct reclaim used to scan highmem even if the allocating
> process did not specify __GFP_HIGHMEM but not any more. kswapd no longer
> will reclaim from zones that are above the high watermark. The intention
> in both cases was to minimise unnecessary reclaim. The downside is on
> machines with large amounts of highmem that lowmem can be fully consumed
> by buffer_heads with nothing trying to free them.
>
> The following patch is based on a suggestion by Andrew Morton to extend
> the buffer_heads_over_limit case to force kswapd and direct reclaim to
> scan the highmem zone regardless of the allocation request or watermarks.
>
> Addresses https://bugzilla.kernel.org/show_bug.cgi?id=42578
>
> ---
>
> diff -Naur linux-3.0/mm/vmscan.c linux-3.0-cc715d99e529d470dde2f33a6614f255adea71f3-backport/mm/vmscan.c
> --- linux-3.0/mm/vmscan.c 2012-04-05 23:09:28.364000004 +0200
> +++ linux-3.0-cc715d99e529d470dde2f33a6614f255adea71f3-backport/mm/vmscan.c 2012-04-05 23:25:30.989968627 +0200
> @@ -1581,6 +1581,14 @@

diff -p is often more helpful, and especially so for this patch:
here we are in shrink_active_list().

> putback_lru_page(page);
> continue;
> }
> +
> + if (unlikely(buffer_heads_over_limit)) {
> + if (page_has_private(page) && trylock_page(page)) {
> + if (page_has_private(page))
> + try_to_release_page(page, 0);
> + unlock_page(page);
> + }
> + }
>
> if (page_referenced(page, 0, sc->mem_cgroup, &vm_flags)) {
> nr_rotated += hpage_nr_pages(page);

I don't think this does functional harm, but it duplicates the work
done by the pagevec_strip() you left in move_active_pages_to_lru(),
so the resulting source would be puzzling.

We could remove the pagevec_strip(), but really, it was just a
misunderstanding that led to my little buffer_heads_over_limit
cleanup in one place getting merged in with Mel's significant
buffer_heads_over_limit fix in another.

My cleanup doesn't deserve backporting to 3.0 or 3.2: I included it
in the 3.3 backport to avoid raised eyebrows, but once we get back
to kernels with pagevec_strip(), let's just leave this hunk out.

> @@ -2053,6 +2061,14 @@

Here we are in all_unreclaimable().

> struct zoneref *z;
> struct zone *zone;
>
> + /*
> + * If the number of buffer_heads in the machine exceeds the maximum
> + * allowed level, force direct reclaim to scan the highmem zone as
> + * highmem pages could be pinning lowmem pages storing buffer_heads
> + */
> + if (buffer_heads_over_limit)
> + sc->gfp_mask |= __GFP_HIGHMEM;
> +

But in Mel's patch that belongs to shrink_zones():
I don't see a reason to move it in the backport.

> for_each_zone_zonelist_nodemask(zone, z, zonelist,
> gfp_zone(sc->gfp_mask), sc->nodemask) {
> if (!populated_zone(zone))
> @@ -2514,7 +2530,8 @@

I think this hunk in balance_pgdat() is correct.

> (zone->present_pages +
> KSWAPD_ZONE_BALANCE_GAP_RATIO-1) /
> KSWAPD_ZONE_BALANCE_GAP_RATIO);
> - if (!zone_watermark_ok_safe(zone, order,
> + if ((buffer_heads_over_limit && is_highmem_idx(i)) ||
> + !zone_watermark_ok_safe(zone, order,
> high_wmark_pages(zone) + balance_gap,
> end_zone, 0)) {
> shrink_zone(priority, zone, &sc);
> @@ -2543,6 +2560,17 @@

But this hunk in balance_pgdat() comes too late: it should set
end_zone in between the inactive_anon_is_low shrink_active_list
and the !zone_watermark_ok_safe() setting of end_zone higher up,
before the previous hunk.

> continue;
> }
>
> + /*
> + * If the number of buffer_heads in the machine
> + * exceeds the maximum allowed level and this node
> + * has a highmem zone, force kswapd to reclaim from
> + * it to relieve lowmem pressure.
> + */
> + if (buffer_heads_over_limit && is_highmem_idx(i)) {
> + end_zone = i;
> + break;
> + }
> +
> if (!zone_watermark_ok_safe(zone, order,
> high_wmark_pages(zone), end_zone, 0)) {
> all_zones_ok = 0;

Hugh


\
 
 \ /
  Last update: 2012-04-07 04:03    [W:0.098 / U:0.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site