Messages in this thread | | | From | (Eric W. Biederman) | Subject | Re: [shm][hugetlb] Fix get_policy for stacked shared memory files | Date | Tue, 12 Jun 2007 00:20:52 -0600 |
| |
Adam Litke <agl@us.ibm.com> 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 <agl@us.ibm.com> > > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |