2021-03-09 15:56:34

by Christoph Hellwig

[permalink] [raw]
Subject: make alloc_anon_inode more useful

Hi all,

this series first renames the existing alloc_anon_inode to
alloc_anon_inode_sb to clearly mark it as requiring a superblock.

It then adds a new alloc_anon_inode that works on the anon_inode
file system super block, thus removing tons of boilerplate code.

The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
later, but might also be ripe for some cleanup.

Diffstat:
arch/powerpc/platforms/pseries/cmm.c | 27 +-------------
drivers/dma-buf/dma-buf.c | 2 -
drivers/gpu/drm/drm_drv.c | 64 +----------------------------------
drivers/misc/cxl/api.c | 2 -
drivers/misc/vmw_balloon.c | 24 +------------
drivers/scsi/cxlflash/ocxl_hw.c | 2 -
drivers/virtio/virtio_balloon.c | 30 +---------------
fs/aio.c | 2 -
fs/anon_inodes.c | 15 +++++++-
fs/libfs.c | 2 -
include/linux/anon_inodes.h | 1
include/linux/fs.h | 2 -
kernel/resource.c | 30 ++--------------
mm/z3fold.c | 38 +-------------------
mm/zsmalloc.c | 48 +-------------------------
15 files changed, 39 insertions(+), 250 deletions(-)


2021-03-09 15:56:35

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

Rename alloc_inode to free the name for a new variant that does not
need boilerplate to create a super_block first.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/powerpc/platforms/pseries/cmm.c | 2 +-
drivers/dma-buf/dma-buf.c | 2 +-
drivers/gpu/drm/drm_drv.c | 2 +-
drivers/misc/cxl/api.c | 2 +-
drivers/misc/vmw_balloon.c | 2 +-
drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
drivers/virtio/virtio_balloon.c | 2 +-
fs/aio.c | 2 +-
fs/anon_inodes.c | 4 ++--
fs/libfs.c | 2 +-
include/linux/fs.h | 2 +-
kernel/resource.c | 2 +-
mm/z3fold.c | 2 +-
mm/zsmalloc.c | 2 +-
14 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
index 45a3a3022a85c9..6d36b858b14df1 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -580,7 +580,7 @@ static int cmm_balloon_compaction_init(void)
return rc;
}

- b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
+ b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
if (IS_ERR(b_dev_info.inode)) {
rc = PTR_ERR(b_dev_info.inode);
b_dev_info.inode = NULL;
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383eb4..dedcc9483352dc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -445,7 +445,7 @@ static inline int is_dma_buf_file(struct file *file)
static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
{
struct file *file;
- struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
+ struct inode *inode = alloc_anon_inode_sb(dma_buf_mnt->mnt_sb);

if (IS_ERR(inode))
return ERR_CAST(inode);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 20d22e41d7ce74..87e7214a8e3565 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -519,7 +519,7 @@ static struct inode *drm_fs_inode_new(void)
return ERR_PTR(r);
}

- inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
+ inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
if (IS_ERR(inode))
simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index b493de962153ba..2efbf6c98028ef 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -73,7 +73,7 @@ static struct file *cxl_getfile(const char *name,
goto err_module;
}

- inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode_sb(cxl_vfs_mount->mnt_sb);
if (IS_ERR(inode)) {
file = ERR_CAST(inode);
goto err_fs;
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index b837e7eba5f7dc..5d057a05ddbee8 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1900,7 +1900,7 @@ static __init int vmballoon_compaction_init(struct vmballoon *b)
return PTR_ERR(vmballoon_mnt);

b->b_dev_info.migratepage = vmballoon_migratepage;
- b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
+ b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);

if (IS_ERR(b->b_dev_info.inode))
return PTR_ERR(b->b_dev_info.inode);
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index 244fc27215dc79..40184ed926b557 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -88,7 +88,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
goto err2;
}

- inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode_sb(ocxlflash_vfs_mount->mnt_sb);
if (IS_ERR(inode)) {
rc = PTR_ERR(inode);
dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8985fc2cea8615..cae76ee5bdd688 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -916,7 +916,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
}

vb->vb_dev_info.migratepage = virtballoon_migratepage;
- vb->vb_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
+ vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
if (IS_ERR(vb->vb_dev_info.inode)) {
err = PTR_ERR(vb->vb_dev_info.inode);
goto out_kern_unmount;
diff --git a/fs/aio.c b/fs/aio.c
index 1f32da13d39ee6..d1c2aa7fd6de7c 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -234,7 +234,7 @@ static const struct address_space_operations aio_ctx_aops;
static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
{
struct file *file;
- struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
+ struct inode *inode = alloc_anon_inode_sb(aio_mnt->mnt_sb);
if (IS_ERR(inode))
return ERR_CAST(inode);

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index a280156138ed89..4745fc37014332 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
const struct qstr qname = QSTR_INIT(name, strlen(name));
int error;

- inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
if (IS_ERR(inode))
return inode;
inode->i_flags &= ~S_PRIVATE;
@@ -231,7 +231,7 @@ static int __init anon_inode_init(void)
if (IS_ERR(anon_inode_mnt))
panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));

- anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
if (IS_ERR(anon_inode_inode))
panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));

diff --git a/fs/libfs.c b/fs/libfs.c
index e2de5401abca5a..600bebc1cd847f 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1216,7 +1216,7 @@ static int anon_set_page_dirty(struct page *page)
return 0;
};

-struct inode *alloc_anon_inode(struct super_block *s)
+struct inode *alloc_anon_inode_sb(struct super_block *s)
{
static const struct address_space_operations anon_aops = {
.set_page_dirty = anon_set_page_dirty,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ec8f3ddf4a6aa8..52387368af3c00 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3286,7 +3286,7 @@ extern int simple_write_end(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned copied,
struct page *page, void *fsdata);
extern int always_delete_dentry(const struct dentry *);
-extern struct inode *alloc_anon_inode(struct super_block *);
+extern struct inode *alloc_anon_inode_sb(struct super_block *);
extern int simple_nosetlease(struct file *, long, struct file_lock **, void **);
extern const struct dentry_operations simple_dentry_operations;

diff --git a/kernel/resource.c b/kernel/resource.c
index 627e61b0c12418..0fd091a3f2fc66 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1863,7 +1863,7 @@ static int __init iomem_init_inode(void)
return rc;
}

- inode = alloc_anon_inode(iomem_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode_sb(iomem_vfs_mount->mnt_sb);
if (IS_ERR(inode)) {
rc = PTR_ERR(inode);
pr_err("Cannot allocate inode for iomem: %d\n", rc);
diff --git a/mm/z3fold.c b/mm/z3fold.c
index b5dafa7e44e429..e7cd9298b221f5 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -376,7 +376,7 @@ static void z3fold_unmount(void)
static const struct address_space_operations z3fold_aops;
static int z3fold_register_migration(struct z3fold_pool *pool)
{
- pool->inode = alloc_anon_inode(z3fold_mnt->mnt_sb);
+ pool->inode = alloc_anon_inode_sb(z3fold_mnt->mnt_sb);
if (IS_ERR(pool->inode)) {
pool->inode = NULL;
return 1;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 30c358b7202510..a6449a2ad861de 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -2086,7 +2086,7 @@ static const struct address_space_operations zsmalloc_aops = {

static int zs_register_migration(struct zs_pool *pool)
{
- pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
+ pool->inode = alloc_anon_inode_sb(zsmalloc_mnt->mnt_sb);
if (IS_ERR(pool->inode)) {
pool->inode = NULL;
return 1;
--
2.30.1

2021-03-09 15:56:38

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/powerpc/platforms/pseries/cmm.c | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
index 6d36b858b14df1..9d07e6bea7126c 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -6,6 +6,7 @@
* Author(s): Brian King ([email protected]),
*/

+#include <linux/anon_inodes.h>
#include <linux/ctype.h>
#include <linux/delay.h>
#include <linux/errno.h>
@@ -502,19 +503,6 @@ static struct notifier_block cmm_mem_nb = {
};

#ifdef CONFIG_BALLOON_COMPACTION
-static struct vfsmount *balloon_mnt;
-
-static int cmm_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, PPC_CMM_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type balloon_fs = {
- .name = "ppc-cmm",
- .init_fs_context = cmm_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
static int cmm_migratepage(struct balloon_dev_info *b_dev_info,
struct page *newpage, struct page *page,
enum migrate_mode mode)
@@ -573,19 +561,10 @@ static int cmm_balloon_compaction_init(void)
balloon_devinfo_init(&b_dev_info);
b_dev_info.migratepage = cmm_migratepage;

- balloon_mnt = kern_mount(&balloon_fs);
- if (IS_ERR(balloon_mnt)) {
- rc = PTR_ERR(balloon_mnt);
- balloon_mnt = NULL;
- return rc;
- }
-
- b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
+ b_dev_info.inode = alloc_anon_inode();
if (IS_ERR(b_dev_info.inode)) {
rc = PTR_ERR(b_dev_info.inode);
b_dev_info.inode = NULL;
- kern_unmount(balloon_mnt);
- balloon_mnt = NULL;
return rc;
}

@@ -597,8 +576,6 @@ static void cmm_balloon_compaction_deinit(void)
if (b_dev_info.inode)
iput(b_dev_info.inode);
b_dev_info.inode = NULL;
- kern_unmount(balloon_mnt);
- balloon_mnt = NULL;
}
#else /* CONFIG_BALLOON_COMPACTION */
static int cmm_balloon_compaction_init(void)
--
2.30.1

2021-03-09 15:57:40

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/9] drm: remove the drm file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/gpu/drm/drm_drv.c | 64 ++-------------------------------------
1 file changed, 3 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 87e7214a8e3565..af293d76f979e5 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -26,6 +26,7 @@
* DEALINGS IN THE SOFTWARE.
*/

+#include <linux/anon_inodes.h>
#include <linux/debugfs.h>
#include <linux/fs.h>
#include <linux/module.h>
@@ -475,65 +476,6 @@ void drm_dev_unplug(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_dev_unplug);

-/*
- * DRM internal mount
- * We want to be able to allocate our own "struct address_space" to control
- * memory-mappings in VRAM (or stolen RAM, ...). However, core MM does not allow
- * stand-alone address_space objects, so we need an underlying inode. As there
- * is no way to allocate an independent inode easily, we need a fake internal
- * VFS mount-point.
- *
- * The drm_fs_inode_new() function allocates a new inode, drm_fs_inode_free()
- * frees it again. You are allowed to use iget() and iput() to get references to
- * the inode. But each drm_fs_inode_new() call must be paired with exactly one
- * drm_fs_inode_free() call (which does not have to be the last iput()).
- * We use drm_fs_inode_*() to manage our internal VFS mount-point and share it
- * between multiple inode-users. You could, technically, call
- * iget() + drm_fs_inode_free() directly after alloc and sometime later do an
- * iput(), but this way you'd end up with a new vfsmount for each inode.
- */
-
-static int drm_fs_cnt;
-static struct vfsmount *drm_fs_mnt;
-
-static int drm_fs_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, 0x010203ff) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type drm_fs_type = {
- .name = "drm",
- .owner = THIS_MODULE,
- .init_fs_context = drm_fs_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
-static struct inode *drm_fs_inode_new(void)
-{
- struct inode *inode;
- int r;
-
- r = simple_pin_fs(&drm_fs_type, &drm_fs_mnt, &drm_fs_cnt);
- if (r < 0) {
- DRM_ERROR("Cannot mount pseudo fs: %d\n", r);
- return ERR_PTR(r);
- }
-
- inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
- if (IS_ERR(inode))
- simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
-
- return inode;
-}
-
-static void drm_fs_inode_free(struct inode *inode)
-{
- if (inode) {
- iput(inode);
- simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
- }
-}
-
/**
* DOC: component helper usage recommendations
*
@@ -563,7 +505,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
{
drm_legacy_ctxbitmap_cleanup(dev);
drm_legacy_remove_map_hash(dev);
- drm_fs_inode_free(dev->anon_inode);
+ iput(dev->anon_inode);

put_device(dev->dev);
/* Prevent use-after-free in drm_managed_release when debugging is
@@ -616,7 +558,7 @@ static int drm_dev_init(struct drm_device *dev,
if (ret)
return ret;

- dev->anon_inode = drm_fs_inode_new();
+ dev->anon_inode = alloc_anon_inode();
if (IS_ERR(dev->anon_inode)) {
ret = PTR_ERR(dev->anon_inode);
DRM_ERROR("Cannot allocate anonymous inode: %d\n", ret);
--
2.30.1

2021-03-09 15:58:06

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/9] vmw_balloon: remove the balloon-vmware file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/misc/vmw_balloon.c | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 5d057a05ddbee8..be4be32f858253 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -16,6 +16,7 @@
//#define DEBUG
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/anon_inodes.h>
#include <linux/types.h>
#include <linux/io.h>
#include <linux/kernel.h>
@@ -1735,20 +1736,6 @@ static inline void vmballoon_debugfs_exit(struct vmballoon *b)


#ifdef CONFIG_BALLOON_COMPACTION
-
-static int vmballoon_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, BALLOON_VMW_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type vmballoon_fs = {
- .name = "balloon-vmware",
- .init_fs_context = vmballoon_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
-static struct vfsmount *vmballoon_mnt;
-
/**
* vmballoon_migratepage() - migrates a balloon page.
* @b_dev_info: balloon device information descriptor.
@@ -1878,8 +1865,6 @@ static void vmballoon_compaction_deinit(struct vmballoon *b)
iput(b->b_dev_info.inode);

b->b_dev_info.inode = NULL;
- kern_unmount(vmballoon_mnt);
- vmballoon_mnt = NULL;
}

/**
@@ -1895,13 +1880,8 @@ static void vmballoon_compaction_deinit(struct vmballoon *b)
*/
static __init int vmballoon_compaction_init(struct vmballoon *b)
{
- vmballoon_mnt = kern_mount(&vmballoon_fs);
- if (IS_ERR(vmballoon_mnt))
- return PTR_ERR(vmballoon_mnt);
-
b->b_dev_info.migratepage = vmballoon_migratepage;
- b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
-
+ b->b_dev_info.inode = alloc_anon_inode();
if (IS_ERR(b->b_dev_info.inode))
return PTR_ERR(b->b_dev_info.inode);

--
2.30.1

2021-03-09 15:58:37

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 7/9] iomem: remove the iomem file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
kernel/resource.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 0fd091a3f2fc66..12560553c26796 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -26,6 +26,7 @@
#include <linux/mm.h>
#include <linux/mount.h>
#include <linux/resource_ext.h>
+#include <linux/anon_inodes.h>
#include <uapi/linux/magic.h>
#include <asm/io.h>

@@ -1838,37 +1839,14 @@ static int __init strict_iomem(char *str)
return 1;
}

-static int iomem_fs_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, DEVMEM_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type iomem_fs_type = {
- .name = "iomem",
- .owner = THIS_MODULE,
- .init_fs_context = iomem_fs_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
static int __init iomem_init_inode(void)
{
- static struct vfsmount *iomem_vfs_mount;
- static int iomem_fs_cnt;
struct inode *inode;
- int rc;
-
- rc = simple_pin_fs(&iomem_fs_type, &iomem_vfs_mount, &iomem_fs_cnt);
- if (rc < 0) {
- pr_err("Cannot mount iomem pseudo filesystem: %d\n", rc);
- return rc;
- }

- inode = alloc_anon_inode_sb(iomem_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode();
if (IS_ERR(inode)) {
- rc = PTR_ERR(inode);
- pr_err("Cannot allocate inode for iomem: %d\n", rc);
- simple_release_fs(&iomem_vfs_mount, &iomem_fs_cnt);
- return rc;
+ pr_err("Cannot allocate inode for iomem: %zd\n", PTR_ERR(inode));
+ return PTR_ERR(inode);
}

/*
--
2.30.1

2021-03-09 15:58:44

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/9] fs: add an argument-less alloc_anon_inode

Add a new alloc_anon_inode helper that allocates an inode on
the anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/anon_inodes.c | 15 +++++++++++++--
include/linux/anon_inodes.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 4745fc37014332..b6a8ea71920bc3 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
const struct qstr qname = QSTR_INIT(name, strlen(name));
int error;

- inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
+ inode = alloc_anon_inode();
if (IS_ERR(inode))
return inode;
inode->i_flags &= ~S_PRIVATE;
@@ -225,13 +225,24 @@ int anon_inode_getfd_secure(const char *name, const struct file_operations *fops
}
EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);

+/**
+ * alloc_anon_inode - create a new anonymous inode
+ *
+ * Create an inode on the anon_inode file system and return it.
+ */
+struct inode *alloc_anon_inode(void)
+{
+ return alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
+}
+EXPORT_SYMBOL_GPL(alloc_anon_inode);
+
static int __init anon_inode_init(void)
{
anon_inode_mnt = kern_mount(&anon_inode_fs_type);
if (IS_ERR(anon_inode_mnt))
panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));

- anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
+ anon_inode_inode = alloc_anon_inode();
if (IS_ERR(anon_inode_inode))
panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));

diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 71881a2b6f7860..b5ae9a6eda9923 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -21,6 +21,7 @@ int anon_inode_getfd_secure(const char *name,
const struct file_operations *fops,
void *priv, int flags,
const struct inode *context_inode);
+struct inode *alloc_anon_inode(void);

#endif /* _LINUX_ANON_INODES_H */

--
2.30.1

2021-03-09 15:59:12

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 8/9] z3fold: remove the z3fold file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
mm/z3fold.c | 38 ++------------------------------------
1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index e7cd9298b221f5..e0749a3d8987de 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -23,6 +23,7 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/anon_inodes.h>
#include <linux/atomic.h>
#include <linux/sched.h>
#include <linux/cpumask.h>
@@ -345,38 +346,10 @@ static inline void free_handle(unsigned long handle, struct z3fold_header *zhdr)
}
}

-static int z3fold_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, Z3FOLD_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type z3fold_fs = {
- .name = "z3fold",
- .init_fs_context = z3fold_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
-static struct vfsmount *z3fold_mnt;
-static int z3fold_mount(void)
-{
- int ret = 0;
-
- z3fold_mnt = kern_mount(&z3fold_fs);
- if (IS_ERR(z3fold_mnt))
- ret = PTR_ERR(z3fold_mnt);
-
- return ret;
-}
-
-static void z3fold_unmount(void)
-{
- kern_unmount(z3fold_mnt);
-}
-
static const struct address_space_operations z3fold_aops;
static int z3fold_register_migration(struct z3fold_pool *pool)
{
- pool->inode = alloc_anon_inode_sb(z3fold_mnt->mnt_sb);
+ pool->inode = alloc_anon_inode();
if (IS_ERR(pool->inode)) {
pool->inode = NULL;
return 1;
@@ -1787,22 +1760,15 @@ MODULE_ALIAS("zpool-z3fold");

static int __init init_z3fold(void)
{
- int ret;
-
/* Make sure the z3fold header is not larger than the page size */
BUILD_BUG_ON(ZHDR_SIZE_ALIGNED > PAGE_SIZE);
- ret = z3fold_mount();
- if (ret)
- return ret;

zpool_register_driver(&z3fold_zpool_driver);
-
return 0;
}

static void __exit exit_z3fold(void)
{
- z3fold_unmount();
zpool_unregister_driver(&z3fold_zpool_driver);
}

--
2.30.1

2021-03-09 16:00:29

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 6/9] virtio_balloon: remove the balloon-kvm file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/virtio/virtio_balloon.c | 30 +++---------------------------
1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index cae76ee5bdd688..1efb890cd3ff09 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -6,6 +6,7 @@
* Copyright 2008 Rusty Russell IBM Corporation
*/

+#include <linux/anon_inodes.h>
#include <linux/virtio.h>
#include <linux/virtio_balloon.h>
#include <linux/swap.h>
@@ -42,10 +43,6 @@
(1 << (VIRTIO_BALLOON_HINT_BLOCK_ORDER + PAGE_SHIFT))
#define VIRTIO_BALLOON_HINT_BLOCK_PAGES (1 << VIRTIO_BALLOON_HINT_BLOCK_ORDER)

-#ifdef CONFIG_BALLOON_COMPACTION
-static struct vfsmount *balloon_mnt;
-#endif
-
enum virtio_balloon_vq {
VIRTIO_BALLOON_VQ_INFLATE,
VIRTIO_BALLOON_VQ_DEFLATE,
@@ -805,18 +802,6 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,

return MIGRATEPAGE_SUCCESS;
}
-
-static int balloon_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, BALLOON_KVM_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type balloon_fs = {
- .name = "balloon-kvm",
- .init_fs_context = balloon_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
#endif /* CONFIG_BALLOON_COMPACTION */

static unsigned long shrink_free_pages(struct virtio_balloon *vb,
@@ -909,17 +894,11 @@ static int virtballoon_probe(struct virtio_device *vdev)
goto out_free_vb;

#ifdef CONFIG_BALLOON_COMPACTION
- balloon_mnt = kern_mount(&balloon_fs);
- if (IS_ERR(balloon_mnt)) {
- err = PTR_ERR(balloon_mnt);
- goto out_del_vqs;
- }
-
vb->vb_dev_info.migratepage = virtballoon_migratepage;
- vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
+ vb->vb_dev_info.inode = alloc_anon_inode();
if (IS_ERR(vb->vb_dev_info.inode)) {
err = PTR_ERR(vb->vb_dev_info.inode);
- goto out_kern_unmount;
+ goto out_del_vqs;
}
vb->vb_dev_info.inode->i_mapping->a_ops = &balloon_aops;
#endif
@@ -1016,8 +995,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
out_iput:
#ifdef CONFIG_BALLOON_COMPACTION
iput(vb->vb_dev_info.inode);
-out_kern_unmount:
- kern_unmount(balloon_mnt);
out_del_vqs:
#endif
vdev->config->del_vqs(vdev);
@@ -1070,7 +1047,6 @@ static void virtballoon_remove(struct virtio_device *vdev)
if (vb->vb_dev_info.inode)
iput(vb->vb_dev_info.inode);

- kern_unmount(balloon_mnt);
#endif
kfree(vb);
}
--
2.30.1

2021-03-09 16:01:06

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 9/9] zsmalloc: remove the zsmalloc file system

Just use the generic anon_inode file system.

Signed-off-by: Christoph Hellwig <[email protected]>
---
mm/zsmalloc.c | 48 +++---------------------------------------------
1 file changed, 3 insertions(+), 45 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index a6449a2ad861de..a7d2f471935447 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -41,6 +41,7 @@
#include <linux/slab.h>
#include <linux/pgtable.h>
#include <asm/tlbflush.h>
+#include <linux/anon_inodes.h>
#include <linux/cpumask.h>
#include <linux/cpu.h>
#include <linux/vmalloc.h>
@@ -176,10 +177,6 @@ struct zs_size_stat {
static struct dentry *zs_stat_root;
#endif

-#ifdef CONFIG_COMPACTION
-static struct vfsmount *zsmalloc_mnt;
-#endif
-
/*
* We assign a page to ZS_ALMOST_EMPTY fullness group when:
* n <= N / f, where
@@ -308,8 +305,6 @@ static void kick_deferred_free(struct zs_pool *pool);
static void init_deferred_free(struct zs_pool *pool);
static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage);
#else
-static int zsmalloc_mount(void) { return 0; }
-static void zsmalloc_unmount(void) {}
static int zs_register_migration(struct zs_pool *pool) { return 0; }
static void zs_unregister_migration(struct zs_pool *pool) {}
static void migrate_lock_init(struct zspage *zspage) {}
@@ -1751,33 +1746,6 @@ static void lock_zspage(struct zspage *zspage)
} while ((page = get_next_page(page)) != NULL);
}

-static int zs_init_fs_context(struct fs_context *fc)
-{
- return init_pseudo(fc, ZSMALLOC_MAGIC) ? 0 : -ENOMEM;
-}
-
-static struct file_system_type zsmalloc_fs = {
- .name = "zsmalloc",
- .init_fs_context = zs_init_fs_context,
- .kill_sb = kill_anon_super,
-};
-
-static int zsmalloc_mount(void)
-{
- int ret = 0;
-
- zsmalloc_mnt = kern_mount(&zsmalloc_fs);
- if (IS_ERR(zsmalloc_mnt))
- ret = PTR_ERR(zsmalloc_mnt);
-
- return ret;
-}
-
-static void zsmalloc_unmount(void)
-{
- kern_unmount(zsmalloc_mnt);
-}
-
static void migrate_lock_init(struct zspage *zspage)
{
rwlock_init(&zspage->lock);
@@ -2086,7 +2054,7 @@ static const struct address_space_operations zsmalloc_aops = {

static int zs_register_migration(struct zs_pool *pool)
{
- pool->inode = alloc_anon_inode_sb(zsmalloc_mnt->mnt_sb);
+ pool->inode = alloc_anon_inode();
if (IS_ERR(pool->inode)) {
pool->inode = NULL;
return 1;
@@ -2506,14 +2474,10 @@ static int __init zs_init(void)
{
int ret;

- ret = zsmalloc_mount();
- if (ret)
- goto out;
-
ret = cpuhp_setup_state(CPUHP_MM_ZS_PREPARE, "mm/zsmalloc:prepare",
zs_cpu_prepare, zs_cpu_dead);
if (ret)
- goto hp_setup_fail;
+ return ret;

#ifdef CONFIG_ZPOOL
zpool_register_driver(&zs_zpool_driver);
@@ -2522,11 +2486,6 @@ static int __init zs_init(void)
zs_stat_init();

return 0;
-
-hp_setup_fail:
- zsmalloc_unmount();
-out:
- return ret;
}

static void __exit zs_exit(void)
@@ -2534,7 +2493,6 @@ static void __exit zs_exit(void)
#ifdef CONFIG_ZPOOL
zpool_unregister_driver(&zs_zpool_driver);
#endif
- zsmalloc_unmount();
cpuhp_remove_state(CPUHP_MM_ZS_PREPARE);

zs_stat_exit();
--
2.30.1

2021-03-09 16:24:09

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

On 09.03.21 16:53, Christoph Hellwig wrote:
> Rename alloc_inode to free the name for a new variant that does not
> need boilerplate to create a super_block first.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/powerpc/platforms/pseries/cmm.c | 2 +-
> drivers/dma-buf/dma-buf.c | 2 +-
> drivers/gpu/drm/drm_drv.c | 2 +-
> drivers/misc/cxl/api.c | 2 +-
> drivers/misc/vmw_balloon.c | 2 +-
> drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
> drivers/virtio/virtio_balloon.c | 2 +-
> fs/aio.c | 2 +-
> fs/anon_inodes.c | 4 ++--
> fs/libfs.c | 2 +-
> include/linux/fs.h | 2 +-
> kernel/resource.c | 2 +-
> mm/z3fold.c | 2 +-
> mm/zsmalloc.c | 2 +-
> 14 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 45a3a3022a85c9..6d36b858b14df1 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -580,7 +580,7 @@ static int cmm_balloon_compaction_init(void)
> return rc;
> }
>
> - b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(b_dev_info.inode)) {
> rc = PTR_ERR(b_dev_info.inode);
> b_dev_info.inode = NULL;
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb4..dedcc9483352dc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -445,7 +445,7 @@ static inline int is_dma_buf_file(struct file *file)
> static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(dma_buf_mnt->mnt_sb);
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 20d22e41d7ce74..87e7214a8e3565 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -519,7 +519,7 @@ static struct inode *drm_fs_inode_new(void)
> return ERR_PTR(r);
> }
>
> - inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
> if (IS_ERR(inode))
> simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index b493de962153ba..2efbf6c98028ef 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -73,7 +73,7 @@ static struct file *cxl_getfile(const char *name,
> goto err_module;
> }
>
> - inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(cxl_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> file = ERR_CAST(inode);
> goto err_fs;
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index b837e7eba5f7dc..5d057a05ddbee8 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -1900,7 +1900,7 @@ static __init int vmballoon_compaction_init(struct vmballoon *b)
> return PTR_ERR(vmballoon_mnt);
>
> b->b_dev_info.migratepage = vmballoon_migratepage;
> - b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
> + b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
>
> if (IS_ERR(b->b_dev_info.inode))
> return PTR_ERR(b->b_dev_info.inode);
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 244fc27215dc79..40184ed926b557 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -88,7 +88,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
> goto err2;
> }
>
> - inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(ocxlflash_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8985fc2cea8615..cae76ee5bdd688 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -916,7 +916,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> }
>
> vb->vb_dev_info.migratepage = virtballoon_migratepage;
> - vb->vb_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(vb->vb_dev_info.inode)) {
> err = PTR_ERR(vb->vb_dev_info.inode);
> goto out_kern_unmount;
> diff --git a/fs/aio.c b/fs/aio.c
> index 1f32da13d39ee6..d1c2aa7fd6de7c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -234,7 +234,7 @@ static const struct address_space_operations aio_ctx_aops;
> static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(aio_mnt->mnt_sb);
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed89..4745fc37014332 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
> const struct qstr qname = QSTR_INIT(name, strlen(name));
> int error;
>
> - inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(inode))
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> @@ -231,7 +231,7 @@ static int __init anon_inode_init(void)
> if (IS_ERR(anon_inode_mnt))
> panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));
>
> - anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(anon_inode_inode))
> panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e2de5401abca5a..600bebc1cd847f 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1216,7 +1216,7 @@ static int anon_set_page_dirty(struct page *page)
> return 0;
> };
>
> -struct inode *alloc_anon_inode(struct super_block *s)
> +struct inode *alloc_anon_inode_sb(struct super_block *s)
> {
> static const struct address_space_operations anon_aops = {
> .set_page_dirty = anon_set_page_dirty,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index ec8f3ddf4a6aa8..52387368af3c00 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3286,7 +3286,7 @@ extern int simple_write_end(struct file *file, struct address_space *mapping,
> loff_t pos, unsigned len, unsigned copied,
> struct page *page, void *fsdata);
> extern int always_delete_dentry(const struct dentry *);
> -extern struct inode *alloc_anon_inode(struct super_block *);
> +extern struct inode *alloc_anon_inode_sb(struct super_block *);
> extern int simple_nosetlease(struct file *, long, struct file_lock **, void **);
> extern const struct dentry_operations simple_dentry_operations;
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 627e61b0c12418..0fd091a3f2fc66 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1863,7 +1863,7 @@ static int __init iomem_init_inode(void)
> return rc;
> }
>
> - inode = alloc_anon_inode(iomem_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(iomem_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> pr_err("Cannot allocate inode for iomem: %d\n", rc);
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index b5dafa7e44e429..e7cd9298b221f5 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -376,7 +376,7 @@ static void z3fold_unmount(void)
> static const struct address_space_operations z3fold_aops;
> static int z3fold_register_migration(struct z3fold_pool *pool)
> {
> - pool->inode = alloc_anon_inode(z3fold_mnt->mnt_sb);
> + pool->inode = alloc_anon_inode_sb(z3fold_mnt->mnt_sb);
> if (IS_ERR(pool->inode)) {
> pool->inode = NULL;
> return 1;
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 30c358b7202510..a6449a2ad861de 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -2086,7 +2086,7 @@ static const struct address_space_operations zsmalloc_aops = {
>
> static int zs_register_migration(struct zs_pool *pool)
> {
> - pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
> + pool->inode = alloc_anon_inode_sb(zsmalloc_mnt->mnt_sb);
> if (IS_ERR(pool->inode)) {
> pool->inode = NULL;
> return 1;
>

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2021-03-09 16:25:10

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 2/9] fs: add an argument-less alloc_anon_inode

On 09.03.21 16:53, Christoph Hellwig wrote:
> Add a new alloc_anon_inode helper that allocates an inode on
> the anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> fs/anon_inodes.c | 15 +++++++++++++--
> include/linux/anon_inodes.h | 1 +
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 4745fc37014332..b6a8ea71920bc3 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
> const struct qstr qname = QSTR_INIT(name, strlen(name));
> int error;
>
> - inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> + inode = alloc_anon_inode();
> if (IS_ERR(inode))
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> @@ -225,13 +225,24 @@ int anon_inode_getfd_secure(const char *name, const struct file_operations *fops
> }
> EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
>
> +/**
> + * alloc_anon_inode - create a new anonymous inode
> + *
> + * Create an inode on the anon_inode file system and return it.
> + */
> +struct inode *alloc_anon_inode(void)
> +{
> + return alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> +}
> +EXPORT_SYMBOL_GPL(alloc_anon_inode);
> +
> static int __init anon_inode_init(void)
> {
> anon_inode_mnt = kern_mount(&anon_inode_fs_type);
> if (IS_ERR(anon_inode_mnt))
> panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));
>
> - anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> + anon_inode_inode = alloc_anon_inode();
> if (IS_ERR(anon_inode_inode))
> panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
>
> diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
> index 71881a2b6f7860..b5ae9a6eda9923 100644
> --- a/include/linux/anon_inodes.h
> +++ b/include/linux/anon_inodes.h
> @@ -21,6 +21,7 @@ int anon_inode_getfd_secure(const char *name,
> const struct file_operations *fops,
> void *priv, int flags,
> const struct inode *context_inode);
> +struct inode *alloc_anon_inode(void);
>
> #endif /* _LINUX_ANON_INODES_H */
>
>

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2021-03-09 16:30:25

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

On 09.03.21 16:53, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/powerpc/platforms/pseries/cmm.c | 27 ++-------------------------
> 1 file changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 6d36b858b14df1..9d07e6bea7126c 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -6,6 +6,7 @@
> * Author(s): Brian King ([email protected]),
> */
>
> +#include <linux/anon_inodes.h>
> #include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/errno.h>
> @@ -502,19 +503,6 @@ static struct notifier_block cmm_mem_nb = {
> };
>
> #ifdef CONFIG_BALLOON_COMPACTION
> -static struct vfsmount *balloon_mnt;
> -
> -static int cmm_init_fs_context(struct fs_context *fc)
> -{
> - return init_pseudo(fc, PPC_CMM_MAGIC) ? 0 : -ENOMEM;
> -}
> -
> -static struct file_system_type balloon_fs = {
> - .name = "ppc-cmm",
> - .init_fs_context = cmm_init_fs_context,
> - .kill_sb = kill_anon_super,
> -};
> -
> static int cmm_migratepage(struct balloon_dev_info *b_dev_info,
> struct page *newpage, struct page *page,
> enum migrate_mode mode)
> @@ -573,19 +561,10 @@ static int cmm_balloon_compaction_init(void)
> balloon_devinfo_init(&b_dev_info);
> b_dev_info.migratepage = cmm_migratepage;
>
> - balloon_mnt = kern_mount(&balloon_fs);
> - if (IS_ERR(balloon_mnt)) {
> - rc = PTR_ERR(balloon_mnt);
> - balloon_mnt = NULL;
> - return rc;
> - }
> -
> - b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> + b_dev_info.inode = alloc_anon_inode();
> if (IS_ERR(b_dev_info.inode)) {
> rc = PTR_ERR(b_dev_info.inode);
> b_dev_info.inode = NULL;
> - kern_unmount(balloon_mnt);
> - balloon_mnt = NULL;
> return rc;
> }
>
> @@ -597,8 +576,6 @@ static void cmm_balloon_compaction_deinit(void)
> if (b_dev_info.inode)
> iput(b_dev_info.inode);
> b_dev_info.inode = NULL;
> - kern_unmount(balloon_mnt);
> - balloon_mnt = NULL;
> }
> #else /* CONFIG_BALLOON_COMPACTION */
> static int cmm_balloon_compaction_init(void)
>

I always wondered why that was necessary after all (with my limited fs
knowledge :) ).

a) I assume you want to remove PPC_CMM_MAGIC from
include/uapi/linux/magic.h as well?

b) Do we still need #include <linux/magic.h>, #include <linux/mount.h>
and #include <linux/pseudo_fs.h>?

Apart from that looks much cleaner.

--
Thanks,

David / dhildenb

2021-03-09 16:30:48

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 5/9] vmw_balloon: remove the balloon-vmware file system

On 09.03.21 16:53, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/misc/vmw_balloon.c | 24 ++----------------------
> 1 file changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index 5d057a05ddbee8..be4be32f858253 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -16,6 +16,7 @@
> //#define DEBUG
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/anon_inodes.h>
> #include <linux/types.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> @@ -1735,20 +1736,6 @@ static inline void vmballoon_debugfs_exit(struct vmballoon *b)
>
>
> #ifdef CONFIG_BALLOON_COMPACTION
> -
> -static int vmballoon_init_fs_context(struct fs_context *fc)
> -{
> - return init_pseudo(fc, BALLOON_VMW_MAGIC) ? 0 : -ENOMEM;
> -}
> -
> -static struct file_system_type vmballoon_fs = {
> - .name = "balloon-vmware",
> - .init_fs_context = vmballoon_init_fs_context,
> - .kill_sb = kill_anon_super,
> -};
> -
> -static struct vfsmount *vmballoon_mnt;
> -
> /**
> * vmballoon_migratepage() - migrates a balloon page.
> * @b_dev_info: balloon device information descriptor.
> @@ -1878,8 +1865,6 @@ static void vmballoon_compaction_deinit(struct vmballoon *b)
> iput(b->b_dev_info.inode);
>
> b->b_dev_info.inode = NULL;
> - kern_unmount(vmballoon_mnt);
> - vmballoon_mnt = NULL;
> }
>
> /**
> @@ -1895,13 +1880,8 @@ static void vmballoon_compaction_deinit(struct vmballoon *b)
> */
> static __init int vmballoon_compaction_init(struct vmballoon *b)
> {
> - vmballoon_mnt = kern_mount(&vmballoon_fs);
> - if (IS_ERR(vmballoon_mnt))
> - return PTR_ERR(vmballoon_mnt);
> -
> b->b_dev_info.migratepage = vmballoon_migratepage;
> - b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
> -
> + b->b_dev_info.inode = alloc_anon_inode();
> if (IS_ERR(b->b_dev_info.inode))
> return PTR_ERR(b->b_dev_info.inode);
>
>

Same comment regarding BALLOON_VMW_MAGIC and includes (mount.h,
pseudo_fs.h).

Apart from that looks good.

--
Thanks,

David / dhildenb

2021-03-09 16:31:33

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 6/9] virtio_balloon: remove the balloon-kvm file system

On 09.03.21 16:53, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/virtio/virtio_balloon.c | 30 +++---------------------------
> 1 file changed, 3 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index cae76ee5bdd688..1efb890cd3ff09 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -6,6 +6,7 @@
> * Copyright 2008 Rusty Russell IBM Corporation
> */
>
> +#include <linux/anon_inodes.h>
> #include <linux/virtio.h>
> #include <linux/virtio_balloon.h>
> #include <linux/swap.h>
> @@ -42,10 +43,6 @@
> (1 << (VIRTIO_BALLOON_HINT_BLOCK_ORDER + PAGE_SHIFT))
> #define VIRTIO_BALLOON_HINT_BLOCK_PAGES (1 << VIRTIO_BALLOON_HINT_BLOCK_ORDER)
>
> -#ifdef CONFIG_BALLOON_COMPACTION
> -static struct vfsmount *balloon_mnt;
> -#endif
> -
> enum virtio_balloon_vq {
> VIRTIO_BALLOON_VQ_INFLATE,
> VIRTIO_BALLOON_VQ_DEFLATE,
> @@ -805,18 +802,6 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
>
> return MIGRATEPAGE_SUCCESS;
> }
> -
> -static int balloon_init_fs_context(struct fs_context *fc)
> -{
> - return init_pseudo(fc, BALLOON_KVM_MAGIC) ? 0 : -ENOMEM;
> -}
> -
> -static struct file_system_type balloon_fs = {
> - .name = "balloon-kvm",
> - .init_fs_context = balloon_init_fs_context,
> - .kill_sb = kill_anon_super,
> -};
> -
> #endif /* CONFIG_BALLOON_COMPACTION */
>
> static unsigned long shrink_free_pages(struct virtio_balloon *vb,
> @@ -909,17 +894,11 @@ static int virtballoon_probe(struct virtio_device *vdev)
> goto out_free_vb;
>
> #ifdef CONFIG_BALLOON_COMPACTION
> - balloon_mnt = kern_mount(&balloon_fs);
> - if (IS_ERR(balloon_mnt)) {
> - err = PTR_ERR(balloon_mnt);
> - goto out_del_vqs;
> - }
> -
> vb->vb_dev_info.migratepage = virtballoon_migratepage;
> - vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> + vb->vb_dev_info.inode = alloc_anon_inode();
> if (IS_ERR(vb->vb_dev_info.inode)) {
> err = PTR_ERR(vb->vb_dev_info.inode);
> - goto out_kern_unmount;
> + goto out_del_vqs;
> }
> vb->vb_dev_info.inode->i_mapping->a_ops = &balloon_aops;
> #endif
> @@ -1016,8 +995,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
> out_iput:
> #ifdef CONFIG_BALLOON_COMPACTION
> iput(vb->vb_dev_info.inode);
> -out_kern_unmount:
> - kern_unmount(balloon_mnt);
> out_del_vqs:
> #endif
> vdev->config->del_vqs(vdev);
> @@ -1070,7 +1047,6 @@ static void virtballoon_remove(struct virtio_device *vdev)
> if (vb->vb_dev_info.inode)
> iput(vb->vb_dev_info.inode);
>
> - kern_unmount(balloon_mnt);
> #endif
> kfree(vb);
> }
>

... you might know what I am going to say :)

Apart from that LGTM.

--
Thanks,

David / dhildenb

2021-03-09 16:32:05

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> arch/powerpc/platforms/pseries/cmm.c | 27 ++-------------------------
> 1 file changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 6d36b858b14df1..9d07e6bea7126c 100644
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -6,6 +6,7 @@
> * Author(s): Brian King ([email protected]),
> */
>
> +#include <linux/anon_inodes.h>
> #include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/errno.h>
> @@ -502,19 +503,6 @@ static struct notifier_block cmm_mem_nb = {
> };
>
> #ifdef CONFIG_BALLOON_COMPACTION
> -static struct vfsmount *balloon_mnt;
> -
> -static int cmm_init_fs_context(struct fs_context *fc)
> -{
> - return init_pseudo(fc, PPC_CMM_MAGIC) ? 0 : -ENOMEM;

Should we clean these unusued magic constants too?

include/uapi/linux/magic.h:#define PPC_CMM_MAGIC 0xc7571590

Jason

2021-03-09 16:56:38

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: make alloc_anon_inode more useful

On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> Hi all,
>
> this series first renames the existing alloc_anon_inode to
> alloc_anon_inode_sb to clearly mark it as requiring a superblock.
>
> It then adds a new alloc_anon_inode that works on the anon_inode
> file system super block, thus removing tons of boilerplate code.
>
> The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> later, but might also be ripe for some cleanup.

I like it

For a submission plan can we have this on a git branch please? I will
need a copy for RDMA and Alex will need one for vfio..

Thanks,
Jason

2021-03-09 19:35:48

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

On Tue, Mar 09, 2021 at 04:53:40PM +0100, Christoph Hellwig wrote:
> Rename alloc_inode to free the name for a new variant that does not
> need boilerplate to create a super_block first.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---

That is a nice idea as well to avoid sb by introducing an unique
fs...

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2021-03-09 19:39:24

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 2/9] fs: add an argument-less alloc_anon_inode

On Tue, Mar 09, 2021 at 04:53:41PM +0100, Christoph Hellwig wrote:
> Add a new alloc_anon_inode helper that allocates an inode on
> the anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Reviewed-by: Gao Xiang <[email protected]>

Thanks,
Gao Xiang

2021-03-10 04:08:53

by Matthew Wilcox

[permalink] [raw]
Subject: Re: make alloc_anon_inode more useful

On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> this series first renames the existing alloc_anon_inode to
> alloc_anon_inode_sb to clearly mark it as requiring a superblock.
>
> It then adds a new alloc_anon_inode that works on the anon_inode
> file system super block, thus removing tons of boilerplate code.
>
> The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> later, but might also be ripe for some cleanup.

On a somewhat related note, could I get you to look at
drivers/video/fbdev/core/fb_defio.c?

As far as I can tell, there's no need for fb_deferred_io_aops to exist.
We could just set file->f_mapping->a_ops to NULL, and set_page_dirty()
would do the exact same thing this code does (except it would get the
return value correct).

But maybe that would make something else go wrong that distinguishes
between page->mapping being NULL and page->mapping->a_ops->foo being NULL?
Completely untested patch ...

diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index a591d291b231..441ec31d3e4d 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -151,17 +151,6 @@ static const struct vm_operations_struct fb_deferred_io_vm_ops = {
.page_mkwrite = fb_deferred_io_mkwrite,
};

-static int fb_deferred_io_set_page_dirty(struct page *page)
-{
- if (!PageDirty(page))
- SetPageDirty(page);
- return 0;
-}
-
-static const struct address_space_operations fb_deferred_io_aops = {
- .set_page_dirty = fb_deferred_io_set_page_dirty,
-};
-
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
vma->vm_ops = &fb_deferred_io_vm_ops;
@@ -212,14 +201,6 @@ void fb_deferred_io_init(struct fb_info *info)
}
EXPORT_SYMBOL_GPL(fb_deferred_io_init);

-void fb_deferred_io_open(struct fb_info *info,
- struct inode *inode,
- struct file *file)
-{
- file->f_mapping->a_ops = &fb_deferred_io_aops;
-}
-EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-
void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 06f5805de2de..c4ba76359f22 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1415,10 +1415,7 @@ __releases(&info->lock)
if (res)
module_put(info->fbops->owner);
}
-#ifdef CONFIG_FB_DEFERRED_IO
- if (info->fbdefio)
- fb_deferred_io_open(info, inode, file);
-#endif
+ file->f_mapping->a_ops = NULL;
out:
unlock_fb_info(info);
if (res)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ecfbcc0553a5..a8dccd23c249 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -659,9 +659,6 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
/* drivers/video/fb_defio.c */
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
extern void fb_deferred_io_init(struct fb_info *info);
-extern void fb_deferred_io_open(struct fb_info *info,
- struct inode *inode,
- struct file *file);
extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);

2021-03-10 06:41:54

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

On Tue, Mar 09, 2021 at 04:53:40PM +0100, Christoph Hellwig wrote:
> Rename alloc_inode to free the name for a new variant that does not
> need boilerplate to create a super_block first.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/powerpc/platforms/pseries/cmm.c | 2 +-
> drivers/dma-buf/dma-buf.c | 2 +-
> drivers/gpu/drm/drm_drv.c | 2 +-
> drivers/misc/cxl/api.c | 2 +-
> drivers/misc/vmw_balloon.c | 2 +-
> drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
> drivers/virtio/virtio_balloon.c | 2 +-
> fs/aio.c | 2 +-
> fs/anon_inodes.c | 4 ++--
> fs/libfs.c | 2 +-
> include/linux/fs.h | 2 +-
> kernel/resource.c | 2 +-
> mm/z3fold.c | 2 +-
> mm/zsmalloc.c | 2 +-
> 14 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 45a3a3022a85c9..6d36b858b14df1 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -580,7 +580,7 @@ static int cmm_balloon_compaction_init(void)
> return rc;
> }
>
> - b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(b_dev_info.inode)) {
> rc = PTR_ERR(b_dev_info.inode);
> b_dev_info.inode = NULL;
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb4..dedcc9483352dc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -445,7 +445,7 @@ static inline int is_dma_buf_file(struct file *file)
> static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(dma_buf_mnt->mnt_sb);
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 20d22e41d7ce74..87e7214a8e3565 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -519,7 +519,7 @@ static struct inode *drm_fs_inode_new(void)
> return ERR_PTR(r);
> }
>
> - inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
> if (IS_ERR(inode))
> simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index b493de962153ba..2efbf6c98028ef 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -73,7 +73,7 @@ static struct file *cxl_getfile(const char *name,
> goto err_module;
> }
>
> - inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(cxl_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> file = ERR_CAST(inode);
> goto err_fs;
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index b837e7eba5f7dc..5d057a05ddbee8 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -1900,7 +1900,7 @@ static __init int vmballoon_compaction_init(struct vmballoon *b)
> return PTR_ERR(vmballoon_mnt);
>
> b->b_dev_info.migratepage = vmballoon_migratepage;
> - b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
> + b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
>
> if (IS_ERR(b->b_dev_info.inode))
> return PTR_ERR(b->b_dev_info.inode);
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 244fc27215dc79..40184ed926b557 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -88,7 +88,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
> goto err2;
> }
>
> - inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(ocxlflash_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8985fc2cea8615..cae76ee5bdd688 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -916,7 +916,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> }
>
> vb->vb_dev_info.migratepage = virtballoon_migratepage;
> - vb->vb_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(vb->vb_dev_info.inode)) {
> err = PTR_ERR(vb->vb_dev_info.inode);
> goto out_kern_unmount;
> diff --git a/fs/aio.c b/fs/aio.c
> index 1f32da13d39ee6..d1c2aa7fd6de7c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -234,7 +234,7 @@ static const struct address_space_operations aio_ctx_aops;
> static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(aio_mnt->mnt_sb);
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed89..4745fc37014332 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
> const struct qstr qname = QSTR_INIT(name, strlen(name));
> int error;
>
> - inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(inode))
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> @@ -231,7 +231,7 @@ static int __init anon_inode_init(void)
> if (IS_ERR(anon_inode_mnt))
> panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));
>
> - anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(anon_inode_inode))
> panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e2de5401abca5a..600bebc1cd847f 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1216,7 +1216,7 @@ static int anon_set_page_dirty(struct page *page)
> return 0;
> };
>
> -struct inode *alloc_anon_inode(struct super_block *s)
> +struct inode *alloc_anon_inode_sb(struct super_block *s)
> {
> static const struct address_space_operations anon_aops = {
> .set_page_dirty = anon_set_page_dirty,

EXPORT_SYMBOL(alloc_anon_inode_sb) ?

2021-03-10 06:45:29

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH 9/9] zsmalloc: remove the zsmalloc file system

On Tue, Mar 09, 2021 at 04:53:48PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Minchan Kim <[email protected]>

2021-03-10 08:32:20

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

On Tue, Mar 09, 2021 at 10:39:05PM -0800, Minchan Kim wrote:
> > -struct inode *alloc_anon_inode(struct super_block *s)
> > +struct inode *alloc_anon_inode_sb(struct super_block *s)
> > {
> > static const struct address_space_operations anon_aops = {
> > .set_page_dirty = anon_set_page_dirty,
>
> EXPORT_SYMBOL(alloc_anon_inode_sb) ?

Yes, I actually fixed this up shortly after sending the series.

2021-03-10 08:36:25

by Christoph Hellwig

[permalink] [raw]
Subject: Re: make alloc_anon_inode more useful

On Tue, Mar 09, 2021 at 12:54:52PM -0400, Jason Gunthorpe wrote:
> On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> > Hi all,
> >
> > this series first renames the existing alloc_anon_inode to
> > alloc_anon_inode_sb to clearly mark it as requiring a superblock.
> >
> > It then adds a new alloc_anon_inode that works on the anon_inode
> > file system super block, thus removing tons of boilerplate code.
> >
> > The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> > later, but might also be ripe for some cleanup.
>
> I like it
>
> For a submission plan can we have this on a git branch please? I will
> need a copy for RDMA and Alex will need one for vfio..

anon_inode.c stuff seems to mostly go through Al's tree, but also various
others. So best would be if Al has a branch, but I could also set one up.

2021-03-10 08:39:16

by Christoph Hellwig

[permalink] [raw]
Subject: Re: make alloc_anon_inode more useful

On Wed, Mar 10, 2021 at 04:05:45AM +0000, Matthew Wilcox wrote:
> On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> > this series first renames the existing alloc_anon_inode to
> > alloc_anon_inode_sb to clearly mark it as requiring a superblock.
> >
> > It then adds a new alloc_anon_inode that works on the anon_inode
> > file system super block, thus removing tons of boilerplate code.
> >
> > The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> > later, but might also be ripe for some cleanup.
>
> On a somewhat related note, could I get you to look at
> drivers/video/fbdev/core/fb_defio.c?
>
> As far as I can tell, there's no need for fb_deferred_io_aops to exist.
> We could just set file->f_mapping->a_ops to NULL, and set_page_dirty()
> would do the exact same thing this code does (except it would get the
> return value correct).

> But maybe that would make something else go wrong that distinguishes
> between page->mapping being NULL and page->mapping->a_ops->foo being NULL?

I can't find any place in the kernel that treats a NULL aops different
from not having the method it is looking for.

> Completely untested patch ...

the patch looks mostly good to me.

> }
> -#ifdef CONFIG_FB_DEFERRED_IO
> - if (info->fbdefio)
> - fb_deferred_io_open(info, inode, file);
> -#endif
> + file->f_mapping->a_ops = NULL;

But I'd also skip this. Drivers generally do not set aops, but if they
do a funtion like this really should not override it. This will require
an audit of the callers, though.

2021-03-10 09:35:12

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb

On Tue, Mar 09, 2021 at 04:53:40PM +0100, Christoph Hellwig wrote:
> Rename alloc_inode to free the name for a new variant that does not
> need boilerplate to create a super_block first.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---

Looks good (with the metioned fix in
https://lore.kernel.org/lkml/[email protected])

Reviewed-by: Christian Brauner <[email protected]>

> arch/powerpc/platforms/pseries/cmm.c | 2 +-
> drivers/dma-buf/dma-buf.c | 2 +-
> drivers/gpu/drm/drm_drv.c | 2 +-
> drivers/misc/cxl/api.c | 2 +-
> drivers/misc/vmw_balloon.c | 2 +-
> drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
> drivers/virtio/virtio_balloon.c | 2 +-
> fs/aio.c | 2 +-
> fs/anon_inodes.c | 4 ++--
> fs/libfs.c | 2 +-
> include/linux/fs.h | 2 +-
> kernel/resource.c | 2 +-
> mm/z3fold.c | 2 +-
> mm/zsmalloc.c | 2 +-
> 14 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 45a3a3022a85c9..6d36b858b14df1 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -580,7 +580,7 @@ static int cmm_balloon_compaction_init(void)
> return rc;
> }
>
> - b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(b_dev_info.inode)) {
> rc = PTR_ERR(b_dev_info.inode);
> b_dev_info.inode = NULL;
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb4..dedcc9483352dc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -445,7 +445,7 @@ static inline int is_dma_buf_file(struct file *file)
> static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(dma_buf_mnt->mnt_sb);
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 20d22e41d7ce74..87e7214a8e3565 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -519,7 +519,7 @@ static struct inode *drm_fs_inode_new(void)
> return ERR_PTR(r);
> }
>
> - inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
> if (IS_ERR(inode))
> simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index b493de962153ba..2efbf6c98028ef 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -73,7 +73,7 @@ static struct file *cxl_getfile(const char *name,
> goto err_module;
> }
>
> - inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(cxl_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> file = ERR_CAST(inode);
> goto err_fs;
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index b837e7eba5f7dc..5d057a05ddbee8 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -1900,7 +1900,7 @@ static __init int vmballoon_compaction_init(struct vmballoon *b)
> return PTR_ERR(vmballoon_mnt);
>
> b->b_dev_info.migratepage = vmballoon_migratepage;
> - b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
> + b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
>
> if (IS_ERR(b->b_dev_info.inode))
> return PTR_ERR(b->b_dev_info.inode);
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 244fc27215dc79..40184ed926b557 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -88,7 +88,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
> goto err2;
> }
>
> - inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(ocxlflash_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8985fc2cea8615..cae76ee5bdd688 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -916,7 +916,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> }
>
> vb->vb_dev_info.migratepage = virtballoon_migratepage;
> - vb->vb_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(vb->vb_dev_info.inode)) {
> err = PTR_ERR(vb->vb_dev_info.inode);
> goto out_kern_unmount;
> diff --git a/fs/aio.c b/fs/aio.c
> index 1f32da13d39ee6..d1c2aa7fd6de7c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -234,7 +234,7 @@ static const struct address_space_operations aio_ctx_aops;
> static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(aio_mnt->mnt_sb);
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed89..4745fc37014332 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
> const struct qstr qname = QSTR_INIT(name, strlen(name));
> int error;
>
> - inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(inode))
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> @@ -231,7 +231,7 @@ static int __init anon_inode_init(void)
> if (IS_ERR(anon_inode_mnt))
> panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));
>
> - anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(anon_inode_inode))
> panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e2de5401abca5a..600bebc1cd847f 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1216,7 +1216,7 @@ static int anon_set_page_dirty(struct page *page)
> return 0;
> };
>
> -struct inode *alloc_anon_inode(struct super_block *s)
> +struct inode *alloc_anon_inode_sb(struct super_block *s)
> {
> static const struct address_space_operations anon_aops = {
> .set_page_dirty = anon_set_page_dirty,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index ec8f3ddf4a6aa8..52387368af3c00 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3286,7 +3286,7 @@ extern int simple_write_end(struct file *file, struct address_space *mapping,
> loff_t pos, unsigned len, unsigned copied,
> struct page *page, void *fsdata);
> extern int always_delete_dentry(const struct dentry *);
> -extern struct inode *alloc_anon_inode(struct super_block *);
> +extern struct inode *alloc_anon_inode_sb(struct super_block *);
> extern int simple_nosetlease(struct file *, long, struct file_lock **, void **);
> extern const struct dentry_operations simple_dentry_operations;
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 627e61b0c12418..0fd091a3f2fc66 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1863,7 +1863,7 @@ static int __init iomem_init_inode(void)
> return rc;
> }
>
> - inode = alloc_anon_inode(iomem_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(iomem_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> pr_err("Cannot allocate inode for iomem: %d\n", rc);
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index b5dafa7e44e429..e7cd9298b221f5 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -376,7 +376,7 @@ static void z3fold_unmount(void)
> static const struct address_space_operations z3fold_aops;
> static int z3fold_register_migration(struct z3fold_pool *pool)
> {
> - pool->inode = alloc_anon_inode(z3fold_mnt->mnt_sb);
> + pool->inode = alloc_anon_inode_sb(z3fold_mnt->mnt_sb);
> if (IS_ERR(pool->inode)) {
> pool->inode = NULL;
> return 1;
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 30c358b7202510..a6449a2ad861de 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -2086,7 +2086,7 @@ static const struct address_space_operations zsmalloc_aops = {
>
> static int zs_register_migration(struct zs_pool *pool)
> {
> - pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
> + pool->inode = alloc_anon_inode_sb(zsmalloc_mnt->mnt_sb);
> if (IS_ERR(pool->inode)) {
> pool->inode = NULL;
> return 1;
> --
> 2.30.1
>

2021-03-10 09:37:24

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 2/9] fs: add an argument-less alloc_anon_inode

On Tue, Mar 09, 2021 at 04:53:41PM +0100, Christoph Hellwig wrote:
> Add a new alloc_anon_inode helper that allocates an inode on
> the anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---

Looks good!
Reviewed-by: Christian Brauner <[email protected]>

2021-03-10 16:39:04

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.

Umm... The only problem I see here is the lifetime rules for
that module, and that's not something introduced in this patchset.
Said that, looks like the logics around that place is duplicated in
cmm.c, vmw_balloon.c and virtion_balloon.c and I wonder if it would
be better off with a helper in mm/balloon.c to be used for that setup...

2021-03-10 16:42:51

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 4/9] drm: remove the drm file system

On Tue, Mar 09, 2021 at 04:53:43PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.

Are you changing the lifetime rules for that module?

2021-03-11 08:36:43

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/9] drm: remove the drm file system

On Wed, Mar 10, 2021 at 04:32:34PM +0000, Al Viro wrote:
> On Tue, Mar 09, 2021 at 04:53:43PM +0100, Christoph Hellwig wrote:
> > Just use the generic anon_inode file system.
>
> Are you changing the lifetime rules for that module?

The core drm module is pinned by the actual drivers that use the
library functions, so no.

2021-03-11 08:46:19

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

On Wed, Mar 10, 2021 at 04:29:51PM +0000, Al Viro wrote:
> On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote:
> > Just use the generic anon_inode file system.
>
> Umm... The only problem I see here is the lifetime rules for
> that module, and that's not something introduced in this patchset.
> Said that, looks like the logics around that place is duplicated in
> cmm.c, vmw_balloon.c and virtion_balloon.c and I wonder if it would
> be better off with a helper in mm/balloon.c to be used for that setup...

Independ of all other discussions untangling that mess does seem
very useful.

2021-03-14 14:01:00

by Oliver Sang

[permalink] [raw]
Subject: [iomem] e14497b88f: BUG:KASAN:null-ptr-deref_in_alloc_anon_inode



Greeting,

FYI, we noticed the following commit (built with clang-13):

commit: e14497b88f9919aeedd47efb2762dfa5fc6b640e ("[PATCH 7/9] iomem: remove the iomem file system")
url: https://github.com/0day-ci/linux/commits/Christoph-Hellwig/fs-rename-alloc_anon_inode-to-alloc_anon_inode_sb/20210310-005356
base: https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-misc.git 080951f99de1e483a9a48f34c079b634f2912a54

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+----------------------------------------------+------------+------------+
| | 0befbcb842 | e14497b88f |
+----------------------------------------------+------------+------------+
| BUG:KASAN:null-ptr-deref_in_alloc_anon_inode | 0 | 12 |
| RIP:alloc_anon_inode | 0 | 12 |
+----------------------------------------------+------------+------------+


If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 3.359173] BUG: KASAN: null-ptr-deref in alloc_anon_inode (kbuild/src/consumer/fs/anon_inodes.c:235)
[ 3.359395] Read of size 8 at addr 0000000000000008 by task swapper/0/1
[ 3.359395]
[ 3.359395] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc2-00012-ge14497b88f99 #2
[ 3.359395] Call Trace:
[ 3.359395] dump_stack (kbuild/src/consumer/include/linux/instrumented.h:86 kbuild/src/consumer/include/asm-generic/atomic-instrumented.h:45 kbuild/src/consumer/lib/dump_stack.c:123)
[ 3.359395] kasan_report (kbuild/src/consumer/mm/kasan/report.c:403 kbuild/src/consumer/mm/kasan/report.c:416)
[ 3.359395] ? amd_cache_northbridges (kbuild/src/consumer/arch/x86/kernel/amd_nb.c:240)
[ 3.359395] ? alloc_anon_inode (kbuild/src/consumer/fs/anon_inodes.c:235)
[ 3.359395] ? reserve_setup (kbuild/src/consumer/kernel/resource.c:1843)
[ 3.359395] __asan_load8 (kbuild/src/consumer/mm/kasan/generic.c:253)
[ 3.359395] alloc_anon_inode (kbuild/src/consumer/fs/anon_inodes.c:235)
[ 3.359395] iomem_init_inode (kbuild/src/consumer/kernel/resource.c:1846)
[ 3.359395] do_one_initcall (kbuild/src/consumer/init/main.c:1226)
[ 3.359395] ? next_arg (kbuild/src/consumer/lib/cmdline.c:257)
[ 3.359395] ? parse_args (kbuild/src/consumer/kernel/params.c:179)
[ 3.359395] do_initcall_level (kbuild/src/consumer/init/main.c:1298)
[ 3.359395] do_initcalls (kbuild/src/consumer/init/main.c:1312)
[ 3.359395] do_basic_setup (kbuild/src/consumer/init/main.c:1336)
[ 3.359395] kernel_init_freeable (kbuild/src/consumer/init/main.c:1541)
[ 3.359395] ? rest_init (kbuild/src/consumer/init/main.c:1421)
[ 3.359395] kernel_init (kbuild/src/consumer/init/main.c:1426)
[ 3.359395] ? rest_init (kbuild/src/consumer/init/main.c:1421)
[ 3.359395] ret_from_fork (kbuild/src/consumer/arch/x86/entry/entry_64.S:300)
[ 3.359395] ==================================================================
[ 3.359395] Disabling lock debugging due to kernel taint
[ 3.359437] BUG: kernel NULL pointer dereference, address: 0000000000000008
[ 3.360918] #PF: supervisor read access in kernel mode
[ 3.361918] #PF: error_code(0x0000) - not-present page
[ 3.362728] PGD 0 P4D 0
[ 3.362728] Oops: 0000 [#1] SMP KASAN
[ 3.362728] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B 5.12.0-rc2-00012-ge14497b88f99 #2
[ 3.362728] RIP: 0010:alloc_anon_inode (kbuild/src/consumer/fs/anon_inodes.c:235)
[ 3.362728] Code: 71 fe ff ff 5d c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 55 48 89 e5 53 48 8b 1d 54 45 cc 02 48 8d 7b 08 e8 4b 8e f4 ff <48> 8b 7b 08 e8 c2 a5 fc ff 5b 5d c3 66 66 2e 0f 1f 84 00 00 00 00
All code
========
0: 71 fe jno 0x0
2: ff (bad)
3: ff 5d c3 lcall *-0x3d(%rbp)
6: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1)
d: 00 00 00 00
11: 0f 1f 40 00 nopl 0x0(%rax)
15: 55 push %rbp
16: 48 89 e5 mov %rsp,%rbp
19: 53 push %rbx
1a: 48 8b 1d 54 45 cc 02 mov 0x2cc4554(%rip),%rbx # 0x2cc4575
21: 48 8d 7b 08 lea 0x8(%rbx),%rdi
25: e8 4b 8e f4 ff callq 0xfffffffffff48e75
2a:* 48 8b 7b 08 mov 0x8(%rbx),%rdi <-- trapping instruction
2e: e8 c2 a5 fc ff callq 0xfffffffffffca5f5
33: 5b pop %rbx
34: 5d pop %rbp
35: c3 retq
36: 66 data16
37: 66 data16
38: 2e cs
39: 0f .byte 0xf
3a: 1f (bad)
3b: 84 00 test %al,(%rax)
3d: 00 00 add %al,(%rax)
...

Code starting with the faulting instruction
===========================================
0: 48 8b 7b 08 mov 0x8(%rbx),%rdi
4: e8 c2 a5 fc ff callq 0xfffffffffffca5cb
9: 5b pop %rbx
a: 5d pop %rbp
b: c3 retq
c: 66 data16
d: 66 data16
e: 2e cs
f: 0f .byte 0xf
10: 1f (bad)
11: 84 00 test %al,(%rax)
13: 00 00 add %al,(%rax)
...
[ 3.362728] RSP: 0000:ffff8881001afd10 EFLAGS: 00010282
[ 3.362728] RAX: ffff8881001a0001 RBX: 0000000000000000 RCX: ffffffff811b7d0f
[ 3.362728] RDX: dffffc0000000000 RSI: 0000000000000000 RDI: ffffffff83c11c58
[ 3.362728] RBP: ffff8881001afd18 R08: dffffc0000000000 R09: fffffbfff078238c
[ 3.362728] R10: fffffbfff078238c R11: 0000000000000000 R12: 0000000000000000
[ 3.362728] R13: 0000000000000000 R14: ffffffff8435b9c0 R15: ffffffff8361c400
[ 3.362728] FS: 0000000000000000(0000) GS:ffff8881e8600000(0000) knlGS:0000000000000000
[ 3.362728] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3.362728] CR2: 0000000000000008 CR3: 0000000003616000 CR4: 00000000000006b0
[ 3.362728] Call Trace:
[ 3.362728] iomem_init_inode (kbuild/src/consumer/kernel/resource.c:1846)
[ 3.362728] do_one_initcall (kbuild/src/consumer/init/main.c:1226)
[ 3.362728] ? next_arg (kbuild/src/consumer/lib/cmdline.c:257)
[ 3.362728] ? parse_args (kbuild/src/consumer/kernel/params.c:179)
[ 3.362728] do_initcall_level (kbuild/src/consumer/init/main.c:1298)
[ 3.362728] do_initcalls (kbuild/src/consumer/init/main.c:1312)
[ 3.362728] do_basic_setup (kbuild/src/consumer/init/main.c:1336)
[ 3.362728] kernel_init_freeable (kbuild/src/consumer/init/main.c:1541)
[ 3.362728] ? rest_init (kbuild/src/consumer/init/main.c:1421)
[ 3.362728] kernel_init (kbuild/src/consumer/init/main.c:1426)
[ 3.362728] ? rest_init (kbuild/src/consumer/init/main.c:1421)
[ 3.362728] ret_from_fork (kbuild/src/consumer/arch/x86/entry/entry_64.S:300)
[ 3.362728] Modules linked in:
[ 3.362728] CR2: 0000000000000008
[ 3.362728] ---[ end trace e17c94a42475f8e5 ]---
[ 3.362728] RIP: 0010:alloc_anon_inode (kbuild/src/consumer/fs/anon_inodes.c:235)
[ 3.362728] Code: 71 fe ff ff 5d c3 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 55 48 89 e5 53 48 8b 1d 54 45 cc 02 48 8d 7b 08 e8 4b 8e f4 ff <48> 8b 7b 08 e8 c2 a5 fc ff 5b 5d c3 66 66 2e 0f 1f 84 00 00 00 00
All code
========
0: 71 fe jno 0x0
2: ff (bad)
3: ff 5d c3 lcall *-0x3d(%rbp)
6: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1)
d: 00 00 00 00
11: 0f 1f 40 00 nopl 0x0(%rax)
15: 55 push %rbp
16: 48 89 e5 mov %rsp,%rbp
19: 53 push %rbx
1a: 48 8b 1d 54 45 cc 02 mov 0x2cc4554(%rip),%rbx # 0x2cc4575
21: 48 8d 7b 08 lea 0x8(%rbx),%rdi
25: e8 4b 8e f4 ff callq 0xfffffffffff48e75
2a:* 48 8b 7b 08 mov 0x8(%rbx),%rdi <-- trapping instruction
2e: e8 c2 a5 fc ff callq 0xfffffffffffca5f5
33: 5b pop %rbx
34: 5d pop %rbp
35: c3 retq
36: 66 data16
37: 66 data16
38: 2e cs
39: 0f .byte 0xf
3a: 1f (bad)
3b: 84 00 test %al,(%rax)
3d: 00 00 add %al,(%rax)
...

Code starting with the faulting instruction
===========================================
0: 48 8b 7b 08 mov 0x8(%rbx),%rdi
4: e8 c2 a5 fc ff callq 0xfffffffffffca5cb
9: 5b pop %rbx
a: 5d pop %rbp
b: c3 retq
c: 66 data16
d: 66 data16
e: 2e cs
f: 0f .byte 0xf
10: 1f (bad)
11: 84 00 test %al,(%rax)
13: 00 00 add %al,(%rax)


To reproduce:

# build kernel
cd linux
cp config-5.12.0-rc2-00012-ge14497b88f99 .config
make HOSTCC=clang-13 CC=clang-13 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email



---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Thanks,
Oliver Sang


Attachments:
(No filename) (9.69 kB)
config-5.12.0-rc2-00012-ge14497b88f99 (155.56 kB)
job-script (4.62 kB)
dmesg.xz (7.57 kB)
Download all attachments

2023-02-03 09:50:18

by Jingbo Xu

[permalink] [raw]
Subject: Re: make alloc_anon_inode more useful

Hi,

Sorry for digging...

This patch series seems useful for fs developers. I'm not sure its
current status and why it doesn't get merged.


On 3/9/21 11:53 PM, Christoph Hellwig wrote:
> Hi all,
>
> this series first renames the existing alloc_anon_inode to
> alloc_anon_inode_sb to clearly mark it as requiring a superblock.
>
> It then adds a new alloc_anon_inode that works on the anon_inode
> file system super block, thus removing tons of boilerplate code.
>
> The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> later, but might also be ripe for some cleanup.
>
> Diffstat:
> arch/powerpc/platforms/pseries/cmm.c | 27 +-------------
> drivers/dma-buf/dma-buf.c | 2 -
> drivers/gpu/drm/drm_drv.c | 64 +----------------------------------
> drivers/misc/cxl/api.c | 2 -
> drivers/misc/vmw_balloon.c | 24 +------------
> drivers/scsi/cxlflash/ocxl_hw.c | 2 -
> drivers/virtio/virtio_balloon.c | 30 +---------------
> fs/aio.c | 2 -
> fs/anon_inodes.c | 15 +++++++-
> fs/libfs.c | 2 -
> include/linux/anon_inodes.h | 1
> include/linux/fs.h | 2 -
> kernel/resource.c | 30 ++--------------
> mm/z3fold.c | 38 +-------------------
> mm/zsmalloc.c | 48 +-------------------------
> 15 files changed, 39 insertions(+), 250 deletions(-)
>

--
Thanks,
Jingbo