Re: [PATCH] Revert "mm/page_alloc: protect pcp->batch accesses with ACCESS_ONCE"

From: Hekuang
Date: Wed Mar 30 2016 - 21:48:27 EST


hi

å 2016/3/31 9:39, Zefan Li åé:
On 2016/3/31 9:14, Hekuang wrote:
Hi

å 2016/3/30 19:10, Michal Hocko åé:
On Wed 30-03-16 18:51:12, Hekuang wrote:
hi

å 2016/3/30 18:38, Mel Gorman åé:
On Wed, Mar 30, 2016 at 10:22:07AM +0000, He Kuang wrote:
This reverts commit 998d39cb236fe464af86a3492a24d2f67ee1efc2.

When local irq is disabled, a percpu variable does not change, so we can
remove the access macros and let the compiler optimize the code safely.

batch can be changed from other contexts. Why is this safe?

I've mistakenly thought that per_cpu variable can only be accessed by that
cpu.
git blame would point you to 998d39cb236f ("mm/page_alloc: protect
pcp->batch accesses with ACCESS_ONCE"). I haven't looked into the code
deeply to confirm this is still the case but it would be a good lead
that this is not that simple. ACCESS_ONCE resp. {READ,WRITE}_ONCE are
usually quite subtle so I would encourage you or anybody else who try to
remove them to study the code and the history deeper before removing
them.

Thank you for responding, I've read that commit and related articles and not sending
mail casually, though you may think it's a stupid patch. I'm a beginner and I think
sending mails to maillist is a effective way to learn kernel, And, sure i'll be more careful and
well prepared next time :)

pcp->batch can be changed in a different cpu. You may read percpu_pagelist_fraction_sysctl_handler()
to see how that can happen.


OK. got it!