Adam Litke <[EMAIL PROTECTED]> writes: > Here's another breakage as a result of shared memory stacked files :( > > The NUMA policy for a VMA is determined by checking the following (in the > order > given): > > 1) vma->vm_ops->get_policy() (if defined) > 2) vma->vm_policy (if defined) > 3) task->mempolicy (if defined) > 4) Fall back to default_policy > > By switching to stacked files for shared memory, get_policy() is now always > set > to shm_get_policy which is a wrapper function. This causes us to stop at step > 1, which yields NULL for hugetlb instead of task->mempolicy which was the > previous (and correct) result. > > This patch modifies the shm_get_policy() wrapper to maintain steps 1-3 for the > wrapped vm_ops. Andi and Christoph, does this look right to you?
I'm confused. I agree that the behavior you describe is correct. However I only see two code paths were get_policy is called and both of them take a NULL result and change it to task->mempolicy: >From mm/mempolicy.c > long do_get_mempolicy(int *policy, nodemask_t *nmask, > unsigned long addr, unsigned long flags) > { > int err; > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma = NULL; > struct mempolicy *pol = current->mempolicy; > > cpuset_update_task_memory_state(); > if (flags & ~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR)) > return -EINVAL; > if (flags & MPOL_F_ADDR) { > down_read(&mm->mmap_sem); > vma = find_vma_intersection(mm, addr, addr+1); > if (!vma) { > up_read(&mm->mmap_sem); > return -EFAULT; > } > if (vma->vm_ops && vma->vm_ops->get_policy) > pol = vma->vm_ops->get_policy(vma, addr); > else > pol = vma->vm_policy; > } else if (addr) > return -EINVAL; > > if (!pol) > pol = &default_policy; > /* Return effective policy for a VMA */ > static struct mempolicy * get_vma_policy(struct task_struct *task, > struct vm_area_struct *vma, unsigned long addr) > { > struct mempolicy *pol = task->mempolicy; > > if (vma) { > if (vma->vm_ops && vma->vm_ops->get_policy) > pol = vma->vm_ops->get_policy(vma, addr); > else if (vma->vm_policy && > vma->vm_policy->policy != MPOL_DEFAULT) > pol = vma->vm_policy; > } > if (!pol) > pol = &default_policy; > return pol; > } Does this perhaps need to be: > Signed-off-by: Adam Litke <[EMAIL PROTECTED]> > > diff --git a/ipc/shm.c b/ipc/shm.c > index 4fefbad..8d2672d 100644 > --- a/ipc/shm.c > +++ b/ipc/shm.c > @@ -254,8 +254,10 @@ struct mempolicy *shm_get_policy(struct vm_area_struct > *vma, unsigned long addr) + pol = NULL; > > if (sfd->vm_ops->get_policy) > pol = sfd->vm_ops->get_policy(vma, addr); > - else > + else if (vma->vm_policy && vma->vm_policy->policy != MPOL_DEFAULT) > pol = vma->vm_policy; > return pol; > } > #endif Sorry I'm just a little dense at the moment. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/