diff mbox series

[1/2] bcachefs: do not use PF_MEMALLOC_NORECLAIM

Message ID 20240902095203.1559361-2-mhocko@kernel.org (mailing list archive)
State Handled Elsewhere
Headers show
Series remove PF_MEMALLOC_NORECLAIM | expand

Commit Message

Michal Hocko Sept. 2, 2024, 9:51 a.m. UTC
From: Michal Hocko <mhocko@suse.com>

bch2_new_inode relies on PF_MEMALLOC_NORECLAIM to try to allocate a new
inode to achieve GFP_NOWAIT semantic while holding locks. If this
allocation fails it will drop locks and use GFP_NOFS allocation context.

We would like to drop PF_MEMALLOC_NORECLAIM because it is really
dangerous to use if the caller doesn't control the full call chain with
this flag set. E.g. if any of the function down the chain needed
GFP_NOFAIL request the PF_MEMALLOC_NORECLAIM would override this and
cause unexpected failure.

While this is not the case in this particular case using the scoped gfp
semantic is not really needed bacause we can easily pus the allocation
context down the chain without too much clutter.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz> # For vfs changes
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 fs/bcachefs/fs.c          | 14 ++++++--------
 fs/inode.c                |  6 +++---
 include/linux/fs.h        |  7 ++++++-
 include/linux/lsm_hooks.h |  2 +-
 include/linux/security.h  |  4 ++--
 security/security.c       |  8 ++++----
 6 files changed, 22 insertions(+), 19 deletions(-)

Comments

kernel test robot Sept. 5, 2024, 9:28 a.m. UTC | #1
Hi Michal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on tip/sched/core brauner-vfs/vfs.all linus/master v6.11-rc6]
[cannot apply to next-20240904]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Michal-Hocko/bcachefs-do-not-use-PF_MEMALLOC_NORECLAIM/20240902-200126
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240902095203.1559361-2-mhocko%40kernel.org
patch subject: [PATCH 1/2] bcachefs: do not use PF_MEMALLOC_NORECLAIM
config: x86_64-allnoconfig (https://download.01.org/0day-ci/archive/20240905/202409051713.LFVMScXi-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240905/202409051713.LFVMScXi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409051713.LFVMScXi-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/inode.c:157: warning: Function parameter or struct member 'gfp' not described in 'inode_init_always_gfp'
>> fs/inode.c:157: warning: expecting prototype for inode_init_always(). Prototype was for inode_init_always_gfp() instead


vim +157 fs/inode.c

bd9b51e79cb0b8 Al Viro             2014-11-18  147  
2cb1599f9b2ecd David Chinner       2008-10-30  148  /**
6e7c2b4dd36d83 Masahiro Yamada     2017-05-08  149   * inode_init_always - perform inode structure initialisation
0bc02f3fa433a9 Randy Dunlap        2009-01-06  150   * @sb: superblock inode belongs to
0bc02f3fa433a9 Randy Dunlap        2009-01-06  151   * @inode: inode to initialise
2cb1599f9b2ecd David Chinner       2008-10-30  152   *
2cb1599f9b2ecd David Chinner       2008-10-30  153   * These are initializations that need to be done on every inode
2cb1599f9b2ecd David Chinner       2008-10-30  154   * allocation as the fields are not initialised by slab allocation.
2cb1599f9b2ecd David Chinner       2008-10-30  155   */
6185335c11aac8 Michal Hocko        2024-09-02  156  int inode_init_always_gfp(struct super_block *sb, struct inode *inode, gfp_t gfp)
^1da177e4c3f41 Linus Torvalds      2005-04-16 @157  {
6e1d5dcc2bbbe7 Alexey Dobriyan     2009-09-21  158  	static const struct inode_operations empty_iops;
bd9b51e79cb0b8 Al Viro             2014-11-18  159  	static const struct file_operations no_open_fops = {.open = no_open};
^1da177e4c3f41 Linus Torvalds      2005-04-16  160  	struct address_space *const mapping = &inode->i_data;
^1da177e4c3f41 Linus Torvalds      2005-04-16  161  
^1da177e4c3f41 Linus Torvalds      2005-04-16  162  	inode->i_sb = sb;
^1da177e4c3f41 Linus Torvalds      2005-04-16  163  	inode->i_blkbits = sb->s_blocksize_bits;
^1da177e4c3f41 Linus Torvalds      2005-04-16  164  	inode->i_flags = 0;
5a9b911b8a24ed Mateusz Guzik       2024-06-11  165  	inode->i_state = 0;
8019ad13ef7f64 Peter Zijlstra      2020-03-04  166  	atomic64_set(&inode->i_sequence, 0);
^1da177e4c3f41 Linus Torvalds      2005-04-16  167  	atomic_set(&inode->i_count, 1);
^1da177e4c3f41 Linus Torvalds      2005-04-16  168  	inode->i_op = &empty_iops;
bd9b51e79cb0b8 Al Viro             2014-11-18  169  	inode->i_fop = &no_open_fops;
edbb35cc6bdfc3 Eric Biggers        2020-10-30  170  	inode->i_ino = 0;
a78ef704a8dd43 Miklos Szeredi      2011-10-28  171  	inode->__i_nlink = 1;
3ddcd0569cd68f Linus Torvalds      2011-08-06  172  	inode->i_opflags = 0;
d0a5b995a30834 Andreas Gruenbacher 2016-09-29  173  	if (sb->s_xattr)
d0a5b995a30834 Andreas Gruenbacher 2016-09-29  174  		inode->i_opflags |= IOP_XATTR;
92361636e0153b Eric W. Biederman   2012-02-08  175  	i_uid_write(inode, 0);
92361636e0153b Eric W. Biederman   2012-02-08  176  	i_gid_write(inode, 0);
^1da177e4c3f41 Linus Torvalds      2005-04-16  177  	atomic_set(&inode->i_writecount, 0);
^1da177e4c3f41 Linus Torvalds      2005-04-16  178  	inode->i_size = 0;
c75b1d9421f80f Jens Axboe          2017-06-27  179  	inode->i_write_hint = WRITE_LIFE_NOT_SET;
^1da177e4c3f41 Linus Torvalds      2005-04-16  180  	inode->i_blocks = 0;
^1da177e4c3f41 Linus Torvalds      2005-04-16  181  	inode->i_bytes = 0;
^1da177e4c3f41 Linus Torvalds      2005-04-16  182  	inode->i_generation = 0;
^1da177e4c3f41 Linus Torvalds      2005-04-16  183  	inode->i_pipe = NULL;
^1da177e4c3f41 Linus Torvalds      2005-04-16  184  	inode->i_cdev = NULL;
61ba64fc076887 Al Viro             2015-05-02  185  	inode->i_link = NULL;
84e710da2a1dfa Al Viro             2016-04-15  186  	inode->i_dir_seq = 0;
^1da177e4c3f41 Linus Torvalds      2005-04-16  187  	inode->i_rdev = 0;
^1da177e4c3f41 Linus Torvalds      2005-04-16  188  	inode->dirtied_when = 0;
6146f0d5e47ca4 Mimi Zohar          2009-02-04  189
diff mbox series

Patch

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 15fc41e63b6c..d151a2f28d12 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -231,9 +231,9 @@  static struct inode *bch2_alloc_inode(struct super_block *sb)
 	BUG();
 }
 
-static struct bch_inode_info *__bch2_new_inode(struct bch_fs *c)
+static struct bch_inode_info *__bch2_new_inode(struct bch_fs *c, gfp_t gfp)
 {
-	struct bch_inode_info *inode = kmem_cache_alloc(bch2_inode_cache, GFP_NOFS);
+	struct bch_inode_info *inode = kmem_cache_alloc(bch2_inode_cache, gfp);
 	if (!inode)
 		return NULL;
 
@@ -245,7 +245,7 @@  static struct bch_inode_info *__bch2_new_inode(struct bch_fs *c)
 	mutex_init(&inode->ei_quota_lock);
 	memset(&inode->ei_devs_need_flush, 0, sizeof(inode->ei_devs_need_flush));
 
-	if (unlikely(inode_init_always(c->vfs_sb, &inode->v))) {
+	if (unlikely(inode_init_always_gfp(c->vfs_sb, &inode->v, gfp))) {
 		kmem_cache_free(bch2_inode_cache, inode);
 		return NULL;
 	}
@@ -258,12 +258,10 @@  static struct bch_inode_info *__bch2_new_inode(struct bch_fs *c)
  */
 static struct bch_inode_info *bch2_new_inode(struct btree_trans *trans)
 {
-	struct bch_inode_info *inode =
-		memalloc_flags_do(PF_MEMALLOC_NORECLAIM|PF_MEMALLOC_NOWARN,
-				  __bch2_new_inode(trans->c));
+	struct bch_inode_info *inode = __bch2_new_inode(trans->c, GFP_NOWAIT);
 
 	if (unlikely(!inode)) {
-		int ret = drop_locks_do(trans, (inode = __bch2_new_inode(trans->c)) ? 0 : -ENOMEM);
+		int ret = drop_locks_do(trans, (inode = __bch2_new_inode(trans->c, GFP_NOFS)) ? 0 : -ENOMEM);
 		if (ret && inode) {
 			__destroy_inode(&inode->v);
 			kmem_cache_free(bch2_inode_cache, inode);
@@ -328,7 +326,7 @@  __bch2_create(struct mnt_idmap *idmap,
 	if (ret)
 		return ERR_PTR(ret);
 #endif
-	inode = __bch2_new_inode(c);
+	inode = __bch2_new_inode(c, GFP_NOFS);
 	if (unlikely(!inode)) {
 		inode = ERR_PTR(-ENOMEM);
 		goto err;
diff --git a/fs/inode.c b/fs/inode.c
index 86670941884b..a2aabbcffbe4 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -153,7 +153,7 @@  static int no_open(struct inode *inode, struct file *file)
  * These are initializations that need to be done on every inode
  * allocation as the fields are not initialised by slab allocation.
  */
-int inode_init_always(struct super_block *sb, struct inode *inode)
+int inode_init_always_gfp(struct super_block *sb, struct inode *inode, gfp_t gfp)
 {
 	static const struct inode_operations empty_iops;
 	static const struct file_operations no_open_fops = {.open = no_open};
@@ -230,14 +230,14 @@  int inode_init_always(struct super_block *sb, struct inode *inode)
 #endif
 	inode->i_flctx = NULL;
 
-	if (unlikely(security_inode_alloc(inode)))
+	if (unlikely(security_inode_alloc(inode, gfp)))
 		return -ENOMEM;
 
 	this_cpu_inc(nr_inodes);
 
 	return 0;
 }
-EXPORT_SYMBOL(inode_init_always);
+EXPORT_SYMBOL(inode_init_always_gfp);
 
 void free_inode_nonrcu(struct inode *inode)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd34b5755c0b..d46ca71a7855 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3027,7 +3027,12 @@  extern loff_t default_llseek(struct file *file, loff_t offset, int whence);
 
 extern loff_t vfs_llseek(struct file *file, loff_t offset, int whence);
 
-extern int inode_init_always(struct super_block *, struct inode *);
+extern int inode_init_always_gfp(struct super_block *, struct inode *, gfp_t);
+static inline int inode_init_always(struct super_block *sb, struct inode *inode)
+{
+	return inode_init_always_gfp(sb, inode, GFP_NOFS);
+}
+
 extern void inode_init_once(struct inode *);
 extern void address_space_init_once(struct address_space *mapping);
 extern struct inode * igrab(struct inode *);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a2ade0ffe9e7..b08472d64765 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -150,6 +150,6 @@  extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
 		__used __section(".early_lsm_info.init")		\
 		__aligned(sizeof(unsigned long))
 
-extern int lsm_inode_alloc(struct inode *inode);
+extern int lsm_inode_alloc(struct inode *inode, gfp_t gfp);
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/include/linux/security.h b/include/linux/security.h
index 1390f1efb4f0..7c6b9b038a0d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -336,7 +336,7 @@  int security_dentry_create_files_as(struct dentry *dentry, int mode,
 					struct cred *new);
 int security_path_notify(const struct path *path, u64 mask,
 					unsigned int obj_type);
-int security_inode_alloc(struct inode *inode);
+int security_inode_alloc(struct inode *inode, gfp_t gfp);
 void security_inode_free(struct inode *inode);
 int security_inode_init_security(struct inode *inode, struct inode *dir,
 				 const struct qstr *qstr,
@@ -769,7 +769,7 @@  static inline int security_path_notify(const struct path *path, u64 mask,
 	return 0;
 }
 
-static inline int security_inode_alloc(struct inode *inode)
+static inline int security_inode_alloc(struct inode *inode, gfp_t gfp)
 {
 	return 0;
 }
diff --git a/security/security.c b/security/security.c
index 8cee5b6c6e6d..3581262da5ee 100644
--- a/security/security.c
+++ b/security/security.c
@@ -660,14 +660,14 @@  static int lsm_file_alloc(struct file *file)
  *
  * Returns 0, or -ENOMEM if memory can't be allocated.
  */
-int lsm_inode_alloc(struct inode *inode)
+int lsm_inode_alloc(struct inode *inode, gfp_t gfp)
 {
 	if (!lsm_inode_cache) {
 		inode->i_security = NULL;
 		return 0;
 	}
 
-	inode->i_security = kmem_cache_zalloc(lsm_inode_cache, GFP_NOFS);
+	inode->i_security = kmem_cache_zalloc(lsm_inode_cache, gfp);
 	if (inode->i_security == NULL)
 		return -ENOMEM;
 	return 0;
@@ -1582,9 +1582,9 @@  int security_path_notify(const struct path *path, u64 mask,
  *
  * Return: Return 0 if operation was successful.
  */
-int security_inode_alloc(struct inode *inode)
+int security_inode_alloc(struct inode *inode, gfp_t gfp)
 {
-	int rc = lsm_inode_alloc(inode);
+	int rc = lsm_inode_alloc(inode, gfp);
 
 	if (unlikely(rc))
 		return rc;