* [PATCH v2 0/3] ext4: Prevent memory reclaim from re-entering the filesystem and deadlocking @ 2019-12-27 8:05 Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Naoto Kobayashi @ 2019-12-27 8:05 UTC (permalink / raw) To: Theodore Ts'o, Andreas Dilger; +Cc: Naoto Kobayashi, linux-ext4 This series aims to prevent ext4_kvmalloc() from re-entering the filesystem and deadlocking. Although __vmalloc() doesn't support GFP_NOFS[1], all of the user of ext4_kvmalloc are using GFP_NOFS (e.g. fs/ext4/resize.c::add_new_gdb), causing the memory reclaim to re-enter the filesystem. To fix this issue, use memalloc_nfs_save/restore() that get __vmalloc() to drop __GFP_FS. [1] linux-tree/Documentation/core-api/gfp-mask-fs-io.rst Changes since v1: - Add Patch 1 to delete ext4_kvzvalloc() that is not used anywhere. - Add Patch 2 to rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument since all the users of ext4_kvmalloc() are using GFP_NOFS Thanks, Naoto Kobayashi (3): ext4: Delete ext4_kvzvalloc() ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument ext4: Prevent ext4_kvmalloc_nofs() from re-entering the filesystem and deadlocking fs/ext4/ext4.h | 3 +-- fs/ext4/resize.c | 10 ++++------ fs/ext4/super.c | 21 +++++++-------------- fs/ext4/xattr.c | 2 +- 4 files changed, 13 insertions(+), 23 deletions(-) -- 2.16.6 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() 2019-12-27 8:05 [PATCH v2 0/3] ext4: Prevent memory reclaim from re-entering the filesystem and deadlocking Naoto Kobayashi @ 2019-12-27 8:05 ` Naoto Kobayashi 2020-01-09 9:51 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 2019-12-27 8:05 ` [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 3/3] ext4: Prevent ext4_kvmalloc_nofs() from re-entering the filesystem and deadlocking Naoto Kobayashi 2 siblings, 2 replies; 13+ messages in thread From: Naoto Kobayashi @ 2019-12-27 8:05 UTC (permalink / raw) To: tytso, adilger.kernel; +Cc: Naoto Kobayashi, linux-ext4 Since we're not using ext4_kvzalloc(), delete this function. Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> --- fs/ext4/ext4.h | 1 - fs/ext4/super.c | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 61987c106511..b25089e3896d 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2678,7 +2678,6 @@ extern int ext4_seq_options_show(struct seq_file *seq, void *offset); extern int ext4_calculate_overhead(struct super_block *sb); extern void ext4_superblock_csum_set(struct super_block *sb); extern void *ext4_kvmalloc(size_t size, gfp_t flags); -extern void *ext4_kvzalloc(size_t size, gfp_t flags); extern int ext4_alloc_flex_bg_array(struct super_block *sb, ext4_group_t ngroup); extern const char *ext4_decode_error(struct super_block *sb, int errno, diff --git a/fs/ext4/super.c b/fs/ext4/super.c index b205112ca051..83a231dedcbf 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -214,16 +214,6 @@ void *ext4_kvmalloc(size_t size, gfp_t flags) return ret; } -void *ext4_kvzalloc(size_t size, gfp_t flags) -{ - void *ret; - - ret = kzalloc(size, flags | __GFP_NOWARN); - if (!ret) - ret = __vmalloc(size, flags | __GFP_ZERO, PAGE_KERNEL); - return ret; -} - ext4_fsblk_t ext4_block_bitmap(struct super_block *sb, struct ext4_group_desc *bg) { -- 2.16.6 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi @ 2020-01-09 9:51 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 1 sibling, 0 replies; 13+ messages in thread From: Jan Kara @ 2020-01-09 9:51 UTC (permalink / raw) To: Naoto Kobayashi; +Cc: tytso, adilger.kernel, linux-ext4 On Fri 27-12-19 17:05:21, Naoto Kobayashi wrote: > Since we're not using ext4_kvzalloc(), delete this function. > > Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> Looks good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/ext4.h | 1 - > fs/ext4/super.c | 10 ---------- > 2 files changed, 11 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 61987c106511..b25089e3896d 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2678,7 +2678,6 @@ extern int ext4_seq_options_show(struct seq_file *seq, void *offset); > extern int ext4_calculate_overhead(struct super_block *sb); > extern void ext4_superblock_csum_set(struct super_block *sb); > extern void *ext4_kvmalloc(size_t size, gfp_t flags); > -extern void *ext4_kvzalloc(size_t size, gfp_t flags); > extern int ext4_alloc_flex_bg_array(struct super_block *sb, > ext4_group_t ngroup); > extern const char *ext4_decode_error(struct super_block *sb, int errno, > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index b205112ca051..83a231dedcbf 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -214,16 +214,6 @@ void *ext4_kvmalloc(size_t size, gfp_t flags) > return ret; > } > > -void *ext4_kvzalloc(size_t size, gfp_t flags) > -{ > - void *ret; > - > - ret = kzalloc(size, flags | __GFP_NOWARN); > - if (!ret) > - ret = __vmalloc(size, flags | __GFP_ZERO, PAGE_KERNEL); > - return ret; > -} > - > ext4_fsblk_t ext4_block_bitmap(struct super_block *sb, > struct ext4_group_desc *bg) > { > -- > 2.16.6 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi 2020-01-09 9:51 ` Jan Kara @ 2020-01-13 22:32 ` Theodore Y. Ts'o 1 sibling, 0 replies; 13+ messages in thread From: Theodore Y. Ts'o @ 2020-01-13 22:32 UTC (permalink / raw) To: Naoto Kobayashi; +Cc: adilger.kernel, linux-ext4 On Fri, Dec 27, 2019 at 05:05:21PM +0900, Naoto Kobayashi wrote: > Since we're not using ext4_kvzalloc(), delete this function. > > Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument 2019-12-27 8:05 [PATCH v2 0/3] ext4: Prevent memory reclaim from re-entering the filesystem and deadlocking Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi @ 2019-12-27 8:05 ` Naoto Kobayashi 2020-01-09 10:00 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 2019-12-27 8:05 ` [PATCH v2 3/3] ext4: Prevent ext4_kvmalloc_nofs() from re-entering the filesystem and deadlocking Naoto Kobayashi 2 siblings, 2 replies; 13+ messages in thread From: Naoto Kobayashi @ 2019-12-27 8:05 UTC (permalink / raw) To: tytso, adilger.kernel; +Cc: Naoto Kobayashi, linux-ext4 Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument, because ext4_kvmalloc() callers must be under GFP_NOFS (otherwise, they should use generic kvmalloc() helper function). Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> --- fs/ext4/ext4.h | 2 +- fs/ext4/resize.c | 10 ++++------ fs/ext4/super.c | 6 +++--- fs/ext4/xattr.c | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index b25089e3896d..e1bdeffca0ad 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2677,7 +2677,7 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb, extern int ext4_seq_options_show(struct seq_file *seq, void *offset); extern int ext4_calculate_overhead(struct super_block *sb); extern void ext4_superblock_csum_set(struct super_block *sb); -extern void *ext4_kvmalloc(size_t size, gfp_t flags); +extern void *ext4_kvmalloc_nofs(size_t size); extern int ext4_alloc_flex_bg_array(struct super_block *sb, ext4_group_t ngroup); extern const char *ext4_decode_error(struct super_block *sb, int errno, diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index a8c0f2b5b6e1..7998bbe66eed 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -824,9 +824,8 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, if (unlikely(err)) goto errout; - n_group_desc = ext4_kvmalloc((gdb_num + 1) * - sizeof(struct buffer_head *), - GFP_NOFS); + n_group_desc = ext4_kvmalloc_nofs((gdb_num + 1) * + sizeof(struct buffer_head *)); if (!n_group_desc) { err = -ENOMEM; ext4_warning(sb, "not enough memory for %lu groups", @@ -900,9 +899,8 @@ static int add_new_gdb_meta_bg(struct super_block *sb, gdb_bh = ext4_sb_bread(sb, gdblock, 0); if (IS_ERR(gdb_bh)) return PTR_ERR(gdb_bh); - n_group_desc = ext4_kvmalloc((gdb_num + 1) * - sizeof(struct buffer_head *), - GFP_NOFS); + n_group_desc = ext4_kvmalloc_nofs((gdb_num + 1) * + sizeof(struct buffer_head *)); if (!n_group_desc) { brelse(gdb_bh); err = -ENOMEM; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 83a231dedcbf..e8965aa6ecce 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -204,13 +204,13 @@ void ext4_superblock_csum_set(struct super_block *sb) es->s_checksum = ext4_superblock_csum(sb, es); } -void *ext4_kvmalloc(size_t size, gfp_t flags) +void *ext4_kvmalloc_nofs(size_t size) { void *ret; - ret = kmalloc(size, flags | __GFP_NOWARN); + ret = kmalloc(size, GFP_NOFS | __GFP_NOWARN); if (!ret) - ret = __vmalloc(size, flags, PAGE_KERNEL); + ret = __vmalloc(size, GFP_NOFS, PAGE_KERNEL); return ret; } diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 8966a5439a22..d5bc970ef331 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -1456,7 +1456,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value, if (!ce) return NULL; - ea_data = ext4_kvmalloc(value_len, GFP_NOFS); + ea_data = ext4_kvmalloc_nofs(value_len); if (!ea_data) { mb_cache_entry_put(ea_inode_cache, ce); return NULL; -- 2.16.6 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument 2019-12-27 8:05 ` [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument Naoto Kobayashi @ 2020-01-09 10:00 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 1 sibling, 0 replies; 13+ messages in thread From: Jan Kara @ 2020-01-09 10:00 UTC (permalink / raw) To: Naoto Kobayashi; +Cc: tytso, adilger.kernel, linux-ext4 On Fri 27-12-19 17:05:22, Naoto Kobayashi wrote: > Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop > its flags argument, because ext4_kvmalloc() callers must be > under GFP_NOFS (otherwise, they should use generic kvmalloc() > helper function). > > Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> Thanks for the patch but I don't think this or the following patch is actually needed. Did you ever see the deadlock with reclaim you mention in the initial message with a recent kernel? The reason is that in all three call sites of ext4_kvmalloc() in ext4 we have a transaction started (which is the reason for GFP_NOFS there after all) but since commit 81378da64de "jbd2: mark the transaction context with the scope GFP_NOFS context" the transaction machinery takes care of updating reclaim context as needed... So I'd be almost inclined to just drop 'flags' argument from ext4_kvmalloc() instead and if we ever create a callsite for which current memalloc_nofs machinery won't be enough, I'd rather extend that than randomly sprinkle GFP_NOFS flags in the code. Honza > --- > fs/ext4/ext4.h | 2 +- > fs/ext4/resize.c | 10 ++++------ > fs/ext4/super.c | 6 +++--- > fs/ext4/xattr.c | 2 +- > 4 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index b25089e3896d..e1bdeffca0ad 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2677,7 +2677,7 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb, > extern int ext4_seq_options_show(struct seq_file *seq, void *offset); > extern int ext4_calculate_overhead(struct super_block *sb); > extern void ext4_superblock_csum_set(struct super_block *sb); > -extern void *ext4_kvmalloc(size_t size, gfp_t flags); > +extern void *ext4_kvmalloc_nofs(size_t size); > extern int ext4_alloc_flex_bg_array(struct super_block *sb, > ext4_group_t ngroup); > extern const char *ext4_decode_error(struct super_block *sb, int errno, > diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c > index a8c0f2b5b6e1..7998bbe66eed 100644 > --- a/fs/ext4/resize.c > +++ b/fs/ext4/resize.c > @@ -824,9 +824,8 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, > if (unlikely(err)) > goto errout; > > - n_group_desc = ext4_kvmalloc((gdb_num + 1) * > - sizeof(struct buffer_head *), > - GFP_NOFS); > + n_group_desc = ext4_kvmalloc_nofs((gdb_num + 1) * > + sizeof(struct buffer_head *)); > if (!n_group_desc) { > err = -ENOMEM; > ext4_warning(sb, "not enough memory for %lu groups", > @@ -900,9 +899,8 @@ static int add_new_gdb_meta_bg(struct super_block *sb, > gdb_bh = ext4_sb_bread(sb, gdblock, 0); > if (IS_ERR(gdb_bh)) > return PTR_ERR(gdb_bh); > - n_group_desc = ext4_kvmalloc((gdb_num + 1) * > - sizeof(struct buffer_head *), > - GFP_NOFS); > + n_group_desc = ext4_kvmalloc_nofs((gdb_num + 1) * > + sizeof(struct buffer_head *)); > if (!n_group_desc) { > brelse(gdb_bh); > err = -ENOMEM; > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 83a231dedcbf..e8965aa6ecce 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -204,13 +204,13 @@ void ext4_superblock_csum_set(struct super_block *sb) > es->s_checksum = ext4_superblock_csum(sb, es); > } > > -void *ext4_kvmalloc(size_t size, gfp_t flags) > +void *ext4_kvmalloc_nofs(size_t size) > { > void *ret; > > - ret = kmalloc(size, flags | __GFP_NOWARN); > + ret = kmalloc(size, GFP_NOFS | __GFP_NOWARN); > if (!ret) > - ret = __vmalloc(size, flags, PAGE_KERNEL); > + ret = __vmalloc(size, GFP_NOFS, PAGE_KERNEL); > return ret; > } > > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 8966a5439a22..d5bc970ef331 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -1456,7 +1456,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value, > if (!ce) > return NULL; > > - ea_data = ext4_kvmalloc(value_len, GFP_NOFS); > + ea_data = ext4_kvmalloc_nofs(value_len); > if (!ea_data) { > mb_cache_entry_put(ea_inode_cache, ce); > return NULL; > -- > 2.16.6 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument 2019-12-27 8:05 ` [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument Naoto Kobayashi 2020-01-09 10:00 ` Jan Kara @ 2020-01-13 22:32 ` Theodore Y. Ts'o 2020-01-16 12:37 ` Jan Kara 1 sibling, 1 reply; 13+ messages in thread From: Theodore Y. Ts'o @ 2020-01-13 22:32 UTC (permalink / raw) To: Naoto Kobayashi; +Cc: adilger.kernel, linux-ext4 On Fri, Dec 27, 2019 at 05:05:22PM +0900, Naoto Kobayashi wrote: > Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop > its flags argument, because ext4_kvmalloc() callers must be > under GFP_NOFS (otherwise, they should use generic kvmalloc() > helper function). > > Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument 2020-01-13 22:32 ` Theodore Y. Ts'o @ 2020-01-16 12:37 ` Jan Kara 2020-01-16 15:12 ` Theodore Y. Ts'o 0 siblings, 1 reply; 13+ messages in thread From: Jan Kara @ 2020-01-16 12:37 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: Naoto Kobayashi, adilger.kernel, linux-ext4 On Mon 13-01-20 17:32:37, Theodore Y. Ts'o wrote: > On Fri, Dec 27, 2019 at 05:05:22PM +0900, Naoto Kobayashi wrote: > > Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop > > its flags argument, because ext4_kvmalloc() callers must be > > under GFP_NOFS (otherwise, they should use generic kvmalloc() > > helper function). > > > > Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> > > Thanks, applied. Ted, I don't think this patch is needed at all - see my email [1]. Sadly Naoto didn't reply to my question whether he really saw any deadlock / lockdep splat or whether it was just a theoretical concern he had... Honza [1] https://lore.kernel.org/linux-ext4/20200109100007.GC27035@quack2.suse.cz -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument 2020-01-16 12:37 ` Jan Kara @ 2020-01-16 15:12 ` Theodore Y. Ts'o 2020-01-16 15:50 ` [PATCH] ext4: drop ext4_kvmalloc() Theodore Ts'o 0 siblings, 1 reply; 13+ messages in thread From: Theodore Y. Ts'o @ 2020-01-16 15:12 UTC (permalink / raw) To: Jan Kara; +Cc: Naoto Kobayashi, adilger.kernel, linux-ext4 On Thu, Jan 16, 2020 at 01:37:24PM +0100, Jan Kara wrote: > > Ted, I don't think this patch is needed at all - see my email [1]. Sadly > Naoto didn't reply to my question whether he really saw any deadlock / > lockdep splat or whether it was just a theoretical concern he had... Thanks, good point. So what we should do instead is just drop ext4_kvmalloc() entirely. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] ext4: drop ext4_kvmalloc() 2020-01-16 15:12 ` Theodore Y. Ts'o @ 2020-01-16 15:50 ` Theodore Ts'o 2020-01-17 10:30 ` Jan Kara 0 siblings, 1 reply; 13+ messages in thread From: Theodore Ts'o @ 2020-01-16 15:50 UTC (permalink / raw) To: Ext4 Developers List; +Cc: jack, naoto.kobayashi4c, Theodore Ts'o As Jan pointed out[1], as of commit 81378da64de ("jbd2: mark the transaction context with the scope GFP_NOFS context") we use memalloc_nofs_{save,restore}() while a jbd2 handle is active. So ext4_kvmalloc() so we can call allocate using GFP_NOFS is no longer necessary. [1] https://lore.kernel.org/r/20200109100007.GC27035@quack2.suse.cz --- fs/ext4/ext4.h | 1 - fs/ext4/resize.c | 10 ++++------ fs/ext4/super.c | 10 ---------- fs/ext4/xattr.c | 2 +- 4 files changed, 5 insertions(+), 18 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 5e621b0da4da..9a2ee2428ecc 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2740,7 +2740,6 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb, extern int ext4_seq_options_show(struct seq_file *seq, void *offset); extern int ext4_calculate_overhead(struct super_block *sb); extern void ext4_superblock_csum_set(struct super_block *sb); -extern void *ext4_kvmalloc(size_t size, gfp_t flags); extern int ext4_alloc_flex_bg_array(struct super_block *sb, ext4_group_t ngroup); extern const char *ext4_decode_error(struct super_block *sb, int errno, diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index a8c0f2b5b6e1..86a2500ed292 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -824,9 +824,8 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, if (unlikely(err)) goto errout; - n_group_desc = ext4_kvmalloc((gdb_num + 1) * - sizeof(struct buffer_head *), - GFP_NOFS); + n_group_desc = kvmalloc((gdb_num + 1) * sizeof(struct buffer_head *), + GFP_KERNEL); if (!n_group_desc) { err = -ENOMEM; ext4_warning(sb, "not enough memory for %lu groups", @@ -900,9 +899,8 @@ static int add_new_gdb_meta_bg(struct super_block *sb, gdb_bh = ext4_sb_bread(sb, gdblock, 0); if (IS_ERR(gdb_bh)) return PTR_ERR(gdb_bh); - n_group_desc = ext4_kvmalloc((gdb_num + 1) * - sizeof(struct buffer_head *), - GFP_NOFS); + n_group_desc = kvmalloc((gdb_num + 1) * sizeof(struct buffer_head *), + GFP_KERNEL); if (!n_group_desc) { brelse(gdb_bh); err = -ENOMEM; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 84a86d9b790f..ecf36a23e0c4 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -204,16 +204,6 @@ void ext4_superblock_csum_set(struct super_block *sb) es->s_checksum = ext4_superblock_csum(sb, es); } -void *ext4_kvmalloc(size_t size, gfp_t flags) -{ - void *ret; - - ret = kmalloc(size, flags | __GFP_NOWARN); - if (!ret) - ret = __vmalloc(size, flags, PAGE_KERNEL); - return ret; -} - ext4_fsblk_t ext4_block_bitmap(struct super_block *sb, struct ext4_group_desc *bg) { diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 246fbeeb6366..8cac7d95c3ad 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -1456,7 +1456,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value, if (!ce) return NULL; - ea_data = ext4_kvmalloc(value_len, GFP_NOFS); + ea_data = kvmalloc(value_len, GFP_KERNEL); if (!ea_data) { mb_cache_entry_put(ea_inode_cache, ce); return NULL; -- 2.24.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] ext4: drop ext4_kvmalloc() 2020-01-16 15:50 ` [PATCH] ext4: drop ext4_kvmalloc() Theodore Ts'o @ 2020-01-17 10:30 ` Jan Kara 2020-01-17 16:36 ` Theodore Y. Ts'o 0 siblings, 1 reply; 13+ messages in thread From: Jan Kara @ 2020-01-17 10:30 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Ext4 Developers List, jack, naoto.kobayashi4c On Thu 16-01-20 10:50:31, Theodore Ts'o wrote: > As Jan pointed out[1], as of commit 81378da64de ("jbd2: mark the > transaction context with the scope GFP_NOFS context") we use > memalloc_nofs_{save,restore}() while a jbd2 handle is active. So > ext4_kvmalloc() so we can call allocate using GFP_NOFS is no longer > necessary. > > [1] https://lore.kernel.org/r/20200109100007.GC27035@quack2.suse.cz Your signed-off-by is missing but otherwise the patch looks good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/ext4.h | 1 - > fs/ext4/resize.c | 10 ++++------ > fs/ext4/super.c | 10 ---------- > fs/ext4/xattr.c | 2 +- > 4 files changed, 5 insertions(+), 18 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 5e621b0da4da..9a2ee2428ecc 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2740,7 +2740,6 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb, > extern int ext4_seq_options_show(struct seq_file *seq, void *offset); > extern int ext4_calculate_overhead(struct super_block *sb); > extern void ext4_superblock_csum_set(struct super_block *sb); > -extern void *ext4_kvmalloc(size_t size, gfp_t flags); > extern int ext4_alloc_flex_bg_array(struct super_block *sb, > ext4_group_t ngroup); > extern const char *ext4_decode_error(struct super_block *sb, int errno, > diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c > index a8c0f2b5b6e1..86a2500ed292 100644 > --- a/fs/ext4/resize.c > +++ b/fs/ext4/resize.c > @@ -824,9 +824,8 @@ static int add_new_gdb(handle_t *handle, struct inode *inode, > if (unlikely(err)) > goto errout; > > - n_group_desc = ext4_kvmalloc((gdb_num + 1) * > - sizeof(struct buffer_head *), > - GFP_NOFS); > + n_group_desc = kvmalloc((gdb_num + 1) * sizeof(struct buffer_head *), > + GFP_KERNEL); > if (!n_group_desc) { > err = -ENOMEM; > ext4_warning(sb, "not enough memory for %lu groups", > @@ -900,9 +899,8 @@ static int add_new_gdb_meta_bg(struct super_block *sb, > gdb_bh = ext4_sb_bread(sb, gdblock, 0); > if (IS_ERR(gdb_bh)) > return PTR_ERR(gdb_bh); > - n_group_desc = ext4_kvmalloc((gdb_num + 1) * > - sizeof(struct buffer_head *), > - GFP_NOFS); > + n_group_desc = kvmalloc((gdb_num + 1) * sizeof(struct buffer_head *), > + GFP_KERNEL); > if (!n_group_desc) { > brelse(gdb_bh); > err = -ENOMEM; > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 84a86d9b790f..ecf36a23e0c4 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -204,16 +204,6 @@ void ext4_superblock_csum_set(struct super_block *sb) > es->s_checksum = ext4_superblock_csum(sb, es); > } > > -void *ext4_kvmalloc(size_t size, gfp_t flags) > -{ > - void *ret; > - > - ret = kmalloc(size, flags | __GFP_NOWARN); > - if (!ret) > - ret = __vmalloc(size, flags, PAGE_KERNEL); > - return ret; > -} > - > ext4_fsblk_t ext4_block_bitmap(struct super_block *sb, > struct ext4_group_desc *bg) > { > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 246fbeeb6366..8cac7d95c3ad 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -1456,7 +1456,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value, > if (!ce) > return NULL; > > - ea_data = ext4_kvmalloc(value_len, GFP_NOFS); > + ea_data = kvmalloc(value_len, GFP_KERNEL); > if (!ea_data) { > mb_cache_entry_put(ea_inode_cache, ce); > return NULL; > -- > 2.24.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ext4: drop ext4_kvmalloc() 2020-01-17 10:30 ` Jan Kara @ 2020-01-17 16:36 ` Theodore Y. Ts'o 0 siblings, 0 replies; 13+ messages in thread From: Theodore Y. Ts'o @ 2020-01-17 16:36 UTC (permalink / raw) To: Jan Kara; +Cc: Ext4 Developers List, naoto.kobayashi4c On Fri, Jan 17, 2020 at 11:30:48AM +0100, Jan Kara wrote: > On Thu 16-01-20 10:50:31, Theodore Ts'o wrote: > > As Jan pointed out[1], as of commit 81378da64de ("jbd2: mark the > > transaction context with the scope GFP_NOFS context") we use > > memalloc_nofs_{save,restore}() while a jbd2 handle is active. So > > ext4_kvmalloc() so we can call allocate using GFP_NOFS is no longer > > necessary. > > > > [1] https://lore.kernel.org/r/20200109100007.GC27035@quack2.suse.cz > > Your signed-off-by is missing but otherwise the patch looks good to me. You > can add: > > Reviewed-by: Jan Kara <jack@suse.cz> Thanks, applied with my signed-off-by and a Link: trailer. - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] ext4: Prevent ext4_kvmalloc_nofs() from re-entering the filesystem and deadlocking 2019-12-27 8:05 [PATCH v2 0/3] ext4: Prevent memory reclaim from re-entering the filesystem and deadlocking Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument Naoto Kobayashi @ 2019-12-27 8:05 ` Naoto Kobayashi 2 siblings, 0 replies; 13+ messages in thread From: Naoto Kobayashi @ 2019-12-27 8:05 UTC (permalink / raw) To: tytso, adilger.kernel; +Cc: Naoto Kobayashi, linux-ext4 Even if __vmalloc() receives GFP_NOFS, this function allocates data pages and auxiliary structures (e.g. pagetables) with __GFP_FS[1]. To prevent memory reclaim from re-entering the filesystem here and potentially deadlocking, use memalloc_nofs_save() that gets __vmalloc() to drop __GFP_FS. [1] linux-tree/Documentation/core-api/gfp-mask-fs-io.rst Signed-off-by: Naoto Kobayashi <naoto.kobayashi4c@gmail.com> --- fs/ext4/super.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index e8965aa6ecce..7f4c9a43a3f3 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -43,6 +43,7 @@ #include <linux/uaccess.h> #include <linux/iversion.h> #include <linux/unicode.h> +#include <linux/sched/mm.h> #include <linux/kthread.h> #include <linux/freezer.h> @@ -206,11 +207,13 @@ void ext4_superblock_csum_set(struct super_block *sb) void *ext4_kvmalloc_nofs(size_t size) { + unsigned int nofs_flag; void *ret; - ret = kmalloc(size, GFP_NOFS | __GFP_NOWARN); - if (!ret) - ret = __vmalloc(size, GFP_NOFS, PAGE_KERNEL); + /* kvmalloc() does not support GFP_NOFS */ + nofs_flag = memalloc_nofs_save(); + ret = kvmalloc(size, GFP_KERNEL); + memalloc_nofs_restore(nofs_flag); return ret; } -- 2.16.6 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-01-17 16:36 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-27 8:05 [PATCH v2 0/3] ext4: Prevent memory reclaim from re-entering the filesystem and deadlocking Naoto Kobayashi 2019-12-27 8:05 ` [PATCH v2 1/3] ext4: Delete ext4_kvzvalloc() Naoto Kobayashi 2020-01-09 9:51 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 2019-12-27 8:05 ` [PATCH v2 2/3] ext4: Rename ext4_kvmalloc() to ext4_kvmalloc_nofs() and drop its flags argument Naoto Kobayashi 2020-01-09 10:00 ` Jan Kara 2020-01-13 22:32 ` Theodore Y. Ts'o 2020-01-16 12:37 ` Jan Kara 2020-01-16 15:12 ` Theodore Y. Ts'o 2020-01-16 15:50 ` [PATCH] ext4: drop ext4_kvmalloc() Theodore Ts'o 2020-01-17 10:30 ` Jan Kara 2020-01-17 16:36 ` Theodore Y. Ts'o 2019-12-27 8:05 ` [PATCH v2 3/3] ext4: Prevent ext4_kvmalloc_nofs() from re-entering the filesystem and deadlocking Naoto Kobayashi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).