* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.