Re: [RFC][PATCH] Remove cgroup member from struct page (v3)

From: Balbir Singh
Date: Wed Sep 17 2008 - 23:59:20 EST


Andrew Morton wrote:
> On Wed, 17 Sep 2008 16:28:26 -0700 Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> wrote:
>
>> Before trying the sparsemem approach, I tried a radix tree per node,
>> per zone and I seemed to actually get some performance
>> improvement.(1.5% (noise maybe))
>>
>> But please do see and review (tested on my x86_64 box with unixbench
>> and some other simple tests)
>>
>> v4..v3
>> 1. Use a radix tree per node, per zone
>>
>> v3...v2
>> 1. Convert flags to unsigned long
>> 2. Move page_cgroup->lock to a bit spin lock in flags
>>
>> v2...v1
>>
>> 1. Fix a small bug, don't call radix_tree_preload_end(), if preload fails
>>
>> This is a rewrite of a patch I had written long back to remove struct page
>> (I shared the patches with Kamezawa, but never posted them anywhere else).
>> I spent the weekend, cleaning them up for 2.6.27-rc5-mmotm (29 Aug 2008).
>>
>> I've tested the patches on an x86_64 box, I've run a simple test running
>> under the memory control group and the same test running concurrently under
>> two different groups (and creating pressure within their groups).
>>
>> Advantages of the patch
>>
>> 1. It removes the extra pointer in struct page
>>
>> Disadvantages
>>
>> 1. Radix tree lookup is not an O(1) operation, once the page is known
>> getting to the page_cgroup (pc) is a little more expensive now.
>
> Why are we doing this? I can guess, but I'd rather not have to.
>
> a) It's slower.
>
> b) It uses even more memory worst-case.
>
> c) It uses less memory best-case.
>
> someone somewhere decided that (Aa + Bb) / Cc < 1.0. What are the values
> of A, B and C and where did they come from? ;)
>
> (IOW, your changelog is in the category "sucky", along with 90% of the others)

Yes, I agree, to be honest we discussed the reasons on the mailing list and
those should go to the changelog. I'll do that in the next version of the
patches. These are early RFC patches, but the changelog does suck.

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