| Message ID | dffc667-5dc9-a980-dab8-8554eafbda7@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mempolicy: cleanups leading to NUMA mpol without vma | expand |
On Mon, Sep 25, 2023 at 01:22:27AM -0700, Hugh Dickins wrote: > It seems strange that kernfs should be an outlier with a set_policy and > get_policy in its kernfs_vm_ops. Ah, it dates back to v2.6.30's commit > 095160aee954 ("sysfs: fix some bin_vm_ops errors"), when I had crashed > on powerpc's pci_mmap_legacy_page_range() fallback to shmem_zero_setup(). > > Well, that was commendably thorough, to give sysfs-bin a set_policy and > get_policy, just to avoid the way it was coded resulting in EINVAL from > mmap when CONFIG_NUMA; but somehow feels a bit over-the-top to me now. > > It's easier to say that nobody should expect to manage a shmem object's > shared NUMA mempolicy via some kernfs backdoor to that object: delete > that code (and there's no longer an EINVAL from mmap in the NUMA case). > > This then leaves set_policy/get_policy as implemented only by shmem - > though importantly also by SysV SHM, which has to interface with shmem > which implements them, and with SHM_HUGETLB which does not. > > Signed-off-by: Hugh Dickins <hughd@google.com> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 180906c36f51..aaa76410e550 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -429,60 +429,11 @@ static int kernfs_vma_access(struct vm_area_struct *vma, unsigned long addr, return ret; } -#ifdef CONFIG_NUMA -static int kernfs_vma_set_policy(struct vm_area_struct *vma, - struct mempolicy *new) -{ - struct file *file = vma->vm_file; - struct kernfs_open_file *of = kernfs_of(file); - int ret; - - if (!of->vm_ops) - return 0; - - if (!kernfs_get_active(of->kn)) - return -EINVAL; - - ret = 0; - if (of->vm_ops->set_policy) - ret = of->vm_ops->set_policy(vma, new); - - kernfs_put_active(of->kn); - return ret; -} - -static struct mempolicy *kernfs_vma_get_policy(struct vm_area_struct *vma, - unsigned long addr) -{ - struct file *file = vma->vm_file; - struct kernfs_open_file *of = kernfs_of(file); - struct mempolicy *pol; - - if (!of->vm_ops) - return vma->vm_policy; - - if (!kernfs_get_active(of->kn)) - return vma->vm_policy; - - pol = vma->vm_policy; - if (of->vm_ops->get_policy) - pol = of->vm_ops->get_policy(vma, addr); - - kernfs_put_active(of->kn); - return pol; -} - -#endif - static const struct vm_operations_struct kernfs_vm_ops = { .open = kernfs_vma_open, .fault = kernfs_vma_fault, .page_mkwrite = kernfs_vma_page_mkwrite, .access = kernfs_vma_access, -#ifdef CONFIG_NUMA - .set_policy = kernfs_vma_set_policy, - .get_policy = kernfs_vma_get_policy, -#endif }; static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma)
It seems strange that kernfs should be an outlier with a set_policy and get_policy in its kernfs_vm_ops. Ah, it dates back to v2.6.30's commit 095160aee954 ("sysfs: fix some bin_vm_ops errors"), when I had crashed on powerpc's pci_mmap_legacy_page_range() fallback to shmem_zero_setup(). Well, that was commendably thorough, to give sysfs-bin a set_policy and get_policy, just to avoid the way it was coded resulting in EINVAL from mmap when CONFIG_NUMA; but somehow feels a bit over-the-top to me now. It's easier to say that nobody should expect to manage a shmem object's shared NUMA mempolicy via some kernfs backdoor to that object: delete that code (and there's no longer an EINVAL from mmap in the NUMA case). This then leaves set_policy/get_policy as implemented only by shmem - though importantly also by SysV SHM, which has to interface with shmem which implements them, and with SHM_HUGETLB which does not. Signed-off-by: Hugh Dickins <hughd@google.com> --- fs/kernfs/file.c | 49 ------------------------------------------------ 1 file changed, 49 deletions(-)