Re: [PATCH] Fix bug in try_to_free_pages and balance_pgdat when they fail to reclaim pages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Martin Bligh wrote:

The same bug is contained in both try_to_free_pages and balance_pgdat.
On reclaiming the requisite number of pages we correctly set
prev_priority back to DEF_PRIORITY.


AFAIKS, we set prev_priority to the priority at which the zone was
deemed to require no more reclaiming, not DEF_PRIORITY.

However, we ALSO do this even
if we loop over all priorities and fail to reclaim.


If that happens, shouldn't prev_priority be set to 0?

I don't agree the patch is correct.


Setting prev_priority artificially high causes reclaimers to set
distress artificially low, and fail to reclaim mapped pages, when
they are, in fact, under severe memory pressure (their priority
may be as low as 0). This causes the OOM killer to fire incorrectly.

This patch changes that to set prev_priority to 0 instead, if we
fail to reclaim.


We saw problems with this before releasing SLES10 too. See
zone_is_near_oom and other changesets from around that era. I would
like to know what workload was prevented from going OOM with these
changes, but zone_is_near_oom didn't help -- it must have been very
marginal (or there may indeed be a bug somewhere).

Nick
--


Signed-off-by: Martin J. Bligh <[email protected]>



------------------------------------------------------------------------

diff -aurpN -X /home/mbligh/.diff.exclude linux-2.6.18/mm/vmscan.c 2.6.18-prev_reset/mm/vmscan.c
--- linux-2.6.18/mm/vmscan.c	2006-09-20 12:24:42.000000000 -0700
+++ 2.6.18-prev_reset/mm/vmscan.c	2006-10-16 17:23:48.000000000 -0700
@@ -962,7 +962,6 @@ static unsigned long shrink_zones(int pr
unsigned long try_to_free_pages(struct zone **zones, gfp_t gfp_mask)
{
	int priority;
-	int ret = 0;
	unsigned long total_scanned = 0;
	unsigned long nr_reclaimed = 0;
	struct reclaim_state *reclaim_state = current->reclaim_state;
@@ -1000,8 +999,15 @@ unsigned long try_to_free_pages(struct z
		}
		total_scanned += sc.nr_scanned;
		if (nr_reclaimed >= sc.swap_cluster_max) {
-			ret = 1;
-			goto out;
+			for (i = 0; zones[i] != 0; i++) {
+				struct zone *zone = zones[i];
+
+				if (!cpuset_zone_allowed(zone, __GFP_HARDWALL))
+					continue;
+
+				zone->prev_priority = zone->temp_priority;
+			}
+			return 1;
		}

		/*
@@ -1021,16 +1027,15 @@ unsigned long try_to_free_pages(struct z
		if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
			blk_congestion_wait(WRITE, HZ/10);
	}
-out:
	for (i = 0; zones[i] != 0; i++) {
		struct zone *zone = zones[i];

		if (!cpuset_zone_allowed(zone, __GFP_HARDWALL))
			continue;

-		zone->prev_priority = zone->temp_priority;
+		zone->prev_priority = 0;
	}
-	return ret;
+	return 0;
}

/*
@@ -1186,7 +1191,10 @@ out:
	for (i = 0; i < pgdat->nr_zones; i++) {
		struct zone *zone = pgdat->node_zones + i;

-		zone->prev_priority = zone->temp_priority;
+		if (priority < 0)		/* we failed to reclaim */
+			zone->prev_priority = 0;
+		else
+			zone->prev_priority = zone->temp_priority;
	}
	if (!all_zones_ok) {
		cond_resched();


Send instant messages to your online friends http://au.messenger.yahoo.com -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux