Re: [RFC][PATCH] radix-tree based page_cgroup. [6/7] radix-tree based page cgroup

[Hirokazu Takahashi <taka@valinux.co.jp>]

Hi,

I looked into the code a bit and I have some comments.

> Each radix-tree entry contains base address of array of page_cgroup.
> As sparsemem does, this registered base address is subtracted by base_pfn
> for that entry. See sparsemem's logic if unsure.
> 
> Signed-off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

  (snip)

> +#define PCGRP_SHIFT	(8)
> +#define PCGRP_SIZE	(1 << PCGRP_SHIFT)

I wonder where the value of PCGRP_SHIFT comes from.

  (snip)

> +static struct page_cgroup *alloc_init_page_cgroup(unsigned long pfn, int nid,
> +					gfp_t mask)
> +{
> +	int size, order;
> +	struct page *page;
> +
> +	size = PCGRP_SIZE * sizeof(struct page_cgroup);
> +	order = get_order(PAGE_ALIGN(size));

I wonder if this alignment will waste some memory.

> +	page = alloc_pages_node(nid, mask, order);

I think you should make "order" be 0 not to cause extra memory pressure
if possible.

> +	if (!page)
> +		return NULL;
> +
> +	init_page_cgroup(page_address(page), pfn);
> +
> +	return page_address(page);
> +}


Thanks.

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