Re: [PATCH v2] tmpfs not interleaving properly
From: KOSAKI Motohiro <hidden>
Date: 2012-06-20 04:02:26
Also in:
linux-mm, lkml
(6/19/12 7:21 PM), Nathan Zimmer wrote:
On Fri, Jun 01, 2012 at 01:22:15PM -0400, KOSAKI Motohiro wrote:quoted
(6/1/12 10:24 AM), Nathan Zimmer wrote:quoted
On Thu, May 31, 2012 at 04:35:53PM -0400, KOSAKI Motohiro wrote:quoted
(5/31/12 4:25 PM), Andrew Morton wrote:quoted
On Thu, 31 May 2012 16:09:15 -0400 KOSAKI Motohiro[off-list ref] wrote:quoted
quoted
--- a/mm/shmem.c +++ b/mm/shmem.c@@ -929,7 +929,7 @@ static struct page *shmem_alloc_page(gfp_t gfp, /* * alloc_page_vma() will drop the shared policy reference */ - return alloc_page_vma(gfp,&pvma, 0); + return alloc_page_vma(gfp,&pvma, info->node_offset<< PAGE_SHIFT );3rd argument of alloc_page_vma() is an address. This is type error.Well, it's an unsigned long... But yes, it is conceptually wrong and *looks* weird. I think we can address that by overcoming our peculair aversion to documenting our code, sigh. This?Sorry, no. addr agrument of alloc_pages_vma() have two meanings. 1) interleave node seed 2) look-up key of shmem policy I think this patch break (2). shmem_get_policy(pol, addr) assume caller honor to pass correct address.But the pseudo vma we generated in shmem_alloc_page the vm_ops are set to NULL. So get_vma_policy will return the policy provided by the pseudo vma and not reach the shmem_get_policy.yes, and it is bug source. we may need to change soon. I guess the right way is to make vm_ops->interleave and interleave_nid uses it if povided.If we provide vm_ops then won't shmem_get_policy get called? That would be an issue since shmem_get_policy assumes vm_file is non NULL.quoted
btw, I don't think node_random() is good idea. it is random(pid + jiffies + cycle). current->cpuset_mem_spread_rotor is per-thread value. but you now need per-inode interleave offset. maybe, just inode addition is enough. Why do you need randomness?I don't really need the randomness, the rotor should be good enough. The correct way to get that is cpuset_mem_spread_node(), yes?
I think that's good idea too.