2023-09-25 10:02:09

by Hugh Dickins

[permalink] [raw]
Subject: [PATCH 02/12] kernfs: drop shared NUMA mempolicy hooks

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 <[email protected]>
---
fs/kernfs/file.c | 49 ------------------------------------------------
1 file changed, 49 deletions(-)

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


2023-09-25 23:17:17

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 02/12] kernfs: drop shared NUMA mempolicy hooks

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 <[email protected]>

Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>