Re: [RFC][PATCH] Clarify mem_cgroup lock handling and avoid races.

[Balbir Singh <balbir@linux.vnet.ibm.com>]

Hugh Dickins wrote:
> On Fri, 22 Feb 2008, Balbir Singh wrote:
>> I've been looking through the code time and again, looking for races. I will try
> 
> Well worth doing.
> 

Yes. I agree 100%. Unfortunately I am not a spin expert and modeling it that way
takes longer than reviewing it a few times.

>> and build a sketch of all the functions and dependencies tonight. One thing that
>> struck me was that making page_get_page_cgroup() call lock_page_cgroup()
>> internally might potentially fix a lot of racy call sites. I was thinking of
>> splitting page_get_page_cgroup into __page_get_page_cgroup() <--> just get the
>> pc without lock and page_get_page_cgroup(), that holds the lock and then returns pc.
> 
> I don't think that would help.  One of the problems with what's there
> (before my patches) is how, for example, clear_page_cgroup takes the
> lock itself - forcing you into dropping the lock before calling it
> (you contemplate keeping an __ which doesn't take the lock, but then
> I cannot see the point).
> 

I just proposed the __ version in case there was a reason. If we can get away
from it, I'll not add __page_get_page_cgroup at all.

> What's there after the patches looks fairly tidy and straightforward
> to me, but emphasize "fairly".  (Often I think there's a race against
> page->page_cgroup going NULL, but then realize that pc->page remains
> stable and there's no such race.)
> 

I agree, I find some of the refactoring very welcome! I did a quick code check
and found that almost instances of cases, where we were worried about pc, called
page_get_page_cgroup() at some point. I thought this might be a good common
place to attack and fix.

>> Of course, this is just a thought process. I am yet to write the code and look
>> at the results.
> 
> I'd hoped to send out my series last night, but was unable to get
> quite that far, sorry, and haven't tested the page migration paths yet.
> The total is not unlike what I already showed, but plus Hirokazu-san's
> patch and minus shmem's NULL page and minus my rearrangement of
> mem_cgroup_charge_common.
> 

Do let me know when you'll have a version to test, I can run LTP, LTP stress and
other tests overnight.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>