Re: [PATCH v3 00/11] mm: fixup changers of per cpu pageset's ->highand ->batch

From: Cody P Schafer
Date: Wed May 01 2013 - 19:53:59 EST


On 04/10/2013 02:25 PM, Cody P Schafer wrote:
On 04/10/2013 02:23 PM, Andrew Morton wrote:
On Wed, 10 Apr 2013 11:23:28 -0700 Cody P Schafer
<cody@xxxxxxxxxxxxxxxxxx> wrote:

"Problems" with the current code:
1. there is a lack of synchronization in setting ->high and ->batch in
percpu_pagelist_fraction_sysctl_handler()
2. stop_machine() in zone_pcp_update() is unnecissary.
3. zone_pcp_update() does not consider the case where
percpu_pagelist_fraction is non-zero

To fix:
1. add memory barriers, a safe ->batch value, an update side mutex
when
updating ->high and ->batch, and use ACCESS_ONCE() for ->batch
users that
expect a stable value.
2. avoid draining pages in zone_pcp_update(), rely upon the memory
barriers added to fix #1
3. factor out quite a few functions, and then call the appropriate
one.

Note that it results in a change to the behavior of
zone_pcp_update(), which is
used by memory_hotplug. I'm rather certain that I've diserned (and
preserved)
the essential behavior (changing ->high and ->batch), and only
eliminated
unneeded actions (draining the per cpu pages), but this may not be
the case.

Further note that the draining of pages that previously took place in
zone_pcp_update() occured after repeated draining when attempting to
offline a
page, and after the offline has "succeeded". It appears that the
draining was
added to zone_pcp_update() to avoid refactoring setup_pageset() into 2
funtions.

There hasn't been a ton of review activity for this patchset :(

I'm inclined to duck it until after 3.9. Do the patches fix any
noticeably bad userspace behavior?

No, all the bugs are theoretical. Waiting should be fine.


Andrew, do you want me to resend this patch set in the hope of obtaining more review? If so, when?

--
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/