kevin/linux
1
Fork 0
Code

bcachefs: do not use PF_MEMALLOC_NORECLAIM

Browse Source 
Patch series "remove PF_MEMALLOC_NORECLAIM" v3.


This patch (of 2):

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.

[akpm@linux-foundation.org: fix kerneldoc warnings]
Link: https://lkml.kernel.org/r/20240926172940.167084-1-mhocko@kernel.org
Link: https://lkml.kernel.org/r/20240926172940.167084-2-mhocko@kernel.org
Signed-off-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz> # For vfs changes
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Yafang Shao <laoar.shao@gmail.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
Michal Hocko 2024-09-26 19:11:50 +02:00 committed by Andrew Morton
parent 8cf0b93919
commit 9897713fe1
5 changed files with 26 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
fs/bcachefs/fs.c
View File

@ -300,10 +300,10 @@ static struct inode *bch2_alloc_inode(struct super_block *sb)
BUG(); 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 = alloc_inode_sb(c->vfs_sb, struct bch_inode_info *inode = alloc_inode_sb(c->vfs_sb,
bch2_inode_cache, GFP_NOFS); bch2_inode_cache, gfp);
if (!inode) if (!inode)
return NULL; return NULL;
@ -315,7 +315,7 @@ static struct bch_inode_info *__bch2_new_inode(struct bch_fs *c)
mutex_init(&inode->ei_quota_lock); mutex_init(&inode->ei_quota_lock);
memset(&inode->ei_devs_need_flush, 0, sizeof(inode->ei_devs_need_flush)); 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); kmem_cache_free(bch2_inode_cache, inode);
return NULL; return NULL;
} }
@ -328,12 +328,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) static struct bch_inode_info *bch2_new_inode(struct btree_trans *trans)
{ {
struct bch_inode_info *inode = struct bch_inode_info *inode = __bch2_new_inode(trans->c, GFP_NOWAIT);
memalloc_flags_do(PF_MEMALLOC_NORECLAIM|PF_MEMALLOC_NOWARN,
__bch2_new_inode(trans->c));
if (unlikely(!inode)) { 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) { if (ret && inode) {
__destroy_inode(&inode->v); __destroy_inode(&inode->v);
kmem_cache_free(bch2_inode_cache, inode); kmem_cache_free(bch2_inode_cache, inode);
@ -407,7 +405,7 @@ __bch2_create(struct mnt_idmap *idmap,
if (ret) if (ret)
return ERR_PTR(ret); return ERR_PTR(ret);
#endif #endif
inode = __bch2_new_inode(c); inode = __bch2_new_inode(c, GFP_NOFS);
if (unlikely(!inode)) { if (unlikely(!inode)) {
inode = ERR_PTR(-ENOMEM); inode = ERR_PTR(-ENOMEM);
goto err; goto err;

10
fs/inode.c
View File

@ -146,14 +146,16 @@ static int no_open(struct inode *inode, struct file *file)
} }
/** /**
* inode_init_always - perform inode structure initialisation * inode_init_always_gfp - perform inode structure initialisation
* @sb: superblock inode belongs to * @sb: superblock inode belongs to
* @inode: inode to initialise * @inode: inode to initialise
* @gfp: allocation flags
* *
* These are initializations that need to be done on every inode * These are initializations that need to be done on every inode
* allocation as the fields are not initialised by slab allocation. * allocation as the fields are not initialised by slab allocation.
* If there are additional allocations required @gfp is used.
*/ */
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 inode_operations empty_iops;
static const struct file_operations no_open_fops = {.open = no_open}; static const struct file_operations no_open_fops = {.open = no_open};
@ -230,14 +232,14 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
#endif #endif
inode->i_flctx = NULL; inode->i_flctx = NULL;
if (unlikely(security_inode_alloc(inode))) if (unlikely(security_inode_alloc(inode, gfp)))
return -ENOMEM; return -ENOMEM;
this_cpu_inc(nr_inodes); this_cpu_inc(nr_inodes);
return 0; return 0;
} }
EXPORT_SYMBOL(inode_init_always); EXPORT_SYMBOL(inode_init_always_gfp);
void free_inode_nonrcu(struct inode *inode) void free_inode_nonrcu(struct inode *inode)
{ {

7
include/linux/fs.h
View File

@ -3082,7 +3082,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 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 inode_init_once(struct inode *);
extern void address_space_init_once(struct address_space *mapping); extern void address_space_init_once(struct address_space *mapping);
extern struct inode * igrab(struct inode *); extern struct inode * igrab(struct inode *);

4
include/linux/security.h
View File

@ -348,7 +348,7 @@ int security_dentry_create_files_as(struct dentry *dentry, int mode,
struct cred *new); struct cred *new);
int security_path_notify(const struct path *path, u64 mask, int security_path_notify(const struct path *path, u64 mask,
unsigned int obj_type); 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); void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir, int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const struct qstr *qstr,
@ -789,7 +789,7 @@ static inline int security_path_notify(const struct path *path, u64 mask,
return 0; 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; return 0;
} }

10
security/security.c
View File

@ -740,19 +740,20 @@ static int lsm_file_alloc(struct file *file)
/** /**
* lsm_inode_alloc - allocate a composite inode blob * lsm_inode_alloc - allocate a composite inode blob
* @inode: the inode that needs a blob * @inode: the inode that needs a blob
* @gfp: allocation flags
* *
* Allocate the inode blob for all the modules * Allocate the inode blob for all the modules
* *
* Returns 0, or -ENOMEM if memory can't be allocated. * Returns 0, or -ENOMEM if memory can't be allocated.
*/ */
static int lsm_inode_alloc(struct inode *inode) static int lsm_inode_alloc(struct inode *inode, gfp_t gfp)
{ {
if (!lsm_inode_cache) { if (!lsm_inode_cache) {
inode->i_security = NULL; inode->i_security = NULL;
return 0; 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) if (inode->i_security == NULL)
return -ENOMEM; return -ENOMEM;
return 0; return 0;
@ -1678,6 +1679,7 @@ int security_path_notify(const struct path *path, u64 mask,
/** /**
* security_inode_alloc() - Allocate an inode LSM blob * security_inode_alloc() - Allocate an inode LSM blob
* @inode: the inode * @inode: the inode
* @gfp: allocation flags
* *
* Allocate and attach a security structure to @inode->i_security. The * Allocate and attach a security structure to @inode->i_security. The
* i_security field is initialized to NULL when the inode structure is * i_security field is initialized to NULL when the inode structure is
@ -1685,9 +1687,9 @@ int security_path_notify(const struct path *path, u64 mask,
* *
* Return: Return 0 if operation was successful. * 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)) if (unlikely(rc))
return rc; return rc;