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

[Nick Piggin]

On Monday 01 September 2008 10:01, KAMEZAWA Hiroyuki wrote:
> On Sun, 31 Aug 2008 23:17:56 +0530
>
> Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> wrote:
> > 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).
>
> It's just because I think there is no strong requirements for 64bit
> count/mapcount. There is no ZERO_PAGE() for ANON (by Nick Piggin. I add him
> to CC.) (shmem still use it but impact is not big.)

I think it would be nice to reduce the impact when it is not configured
anyway. Normally I would not mind so much, but this is something that
many distros will want to enable but fewer users will make use of it.

I think it is always a very good idea to try to reduce struct page size.
When looking at the performance impact though, just be careful with the
alignment of struct page... I actually think it is going to be a
performance win in many cases to make struct page 64 bytes.


> > 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).
> > I've also compiled the patch with CGROUP_MEM_RES_CTLR turned off.
> >
> > Advantages of the patch
> >
> > 1. It removes the extra pointer in struct page
> >
> > Disadvantages
> >
> > 1. It adds an additional lock structure to struct page_cgroup
> > 2. 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.
> >
> > This is an initial RFC for comments
> >
> > TODOs
> >
> > 1. Test the page migration changes
> > 2. Test the performance impact of the patch/approach
> >
> > Comments/Reviews?
>
> plz wait until lockless page cgroup....
>
> And If you don't support radix-tree-delete(), pre-allocating all at boot is
> better.

If you do that, it might even be an idea to allocate flat arrays with
bootmem. It would just be slightly more tricky more tricky to fit this
in with the memory model. But that's not a requirement, just an idea
for a small optimisation.

Thanks,
Nick
--
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/