* [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
* [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
* [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
* 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 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 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
* 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
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).