* [PATCH v2 0/4] ext2: fix race between setxattr and write back
@ 2023-08-15 11:26 Ye Bin
2023-08-15 11:26 ` [PATCH v2 1/4] ext2: remove ext2_new_block() Ye Bin
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Ye Bin @ 2023-08-15 11:26 UTC (permalink / raw)
To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin
Diff v2 vs v1:
1. Do not use reservation window when allocate block for xattr.
2. Remove BUG() in ext2_try_to_allocate_with_rsv().
Ye Bin (4):
ext2: remove ext2_new_block()
ext2: introduce flag argument for ext2_new_blocks()
ext2: fix race between setxattr and write back
ext2: dump current reservation window info
fs/ext2/balloc.c | 32 +++++++++++++++++---------------
fs/ext2/ext2.h | 8 ++++++--
fs/ext2/inode.c | 2 +-
fs/ext2/xattr.c | 4 +++-
4 files changed, 27 insertions(+), 19 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/4] ext2: remove ext2_new_block()
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
@ 2023-08-15 11:26 ` Ye Bin
2023-08-15 11:26 ` [PATCH v2 2/4] ext2: introduce flag argument for ext2_new_blocks() Ye Bin
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2023-08-15 11:26 UTC (permalink / raw)
To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin
Now, only xattr allocate block use ext2_new_block(), so just opencode it in
the xattr code.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext2/balloc.c | 7 -------
fs/ext2/ext2.h | 1 -
fs/ext2/xattr.c | 4 +++-
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index c8049c90323d..c8515fc501ec 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -1429,13 +1429,6 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal,
return 0;
}
-ext2_fsblk_t ext2_new_block(struct inode *inode, unsigned long goal, int *errp)
-{
- unsigned long count = 1;
-
- return ext2_new_blocks(inode, goal, &count, errp);
-}
-
#ifdef EXT2FS_DEBUG
unsigned long ext2_count_free(struct buffer_head *map, unsigned int numchars)
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 35a041c47c38..954fb82ab22c 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -695,7 +695,6 @@ static inline struct ext2_inode_info *EXT2_I(struct inode *inode)
/* balloc.c */
extern int ext2_bg_has_super(struct super_block *sb, int group);
extern unsigned long ext2_bg_num_gdb(struct super_block *sb, int group);
-extern ext2_fsblk_t ext2_new_block(struct inode *, unsigned long, int *);
extern ext2_fsblk_t ext2_new_blocks(struct inode *, unsigned long,
unsigned long *, int *);
extern int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 1c9187188d68..e9546cc65db2 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -742,7 +742,9 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
/* We need to allocate a new block */
ext2_fsblk_t goal = ext2_group_first_block_no(sb,
EXT2_I(inode)->i_block_group);
- int block = ext2_new_block(inode, goal, &error);
+ unsigned long count = 1;
+ int block = ext2_new_blocks(inode, goal, &count,
+ &error);
if (error)
goto cleanup;
ea_idebug(inode, "creating block %d", block);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] ext2: introduce flag argument for ext2_new_blocks()
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
2023-08-15 11:26 ` [PATCH v2 1/4] ext2: remove ext2_new_block() Ye Bin
@ 2023-08-15 11:26 ` Ye Bin
2023-08-15 11:26 ` [PATCH v2 3/4] ext2: fix race between setxattr and write back Ye Bin
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2023-08-15 11:26 UTC (permalink / raw)
To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin
This patch introduce introduce flag argument for ext2_new_blocks(), and also
introduce EXT2_ALLOC_NORESERVE flags.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext2/balloc.c | 3 ++-
fs/ext2/ext2.h | 7 ++++++-
fs/ext2/inode.c | 2 +-
fs/ext2/xattr.c | 2 +-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index c8515fc501ec..99a19f2ecd6f 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -1193,6 +1193,7 @@ int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
* @goal: given target block(filesystem wide)
* @count: target number of blocks to allocate
* @errp: error code
+ * @flag allocate flags
*
* ext2_new_blocks uses a goal block to assist allocation. If the goal is
* free, or there is a free block within 32 blocks of the goal, that block
@@ -1202,7 +1203,7 @@ int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
* This function also updates quota and i_blocks field.
*/
ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal,
- unsigned long *count, int *errp)
+ unsigned long *count, int *errp, unsigned int flag)
{
struct buffer_head *bitmap_bh = NULL;
struct buffer_head *gdp_bh;
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 954fb82ab22c..36c8ed5dd0a0 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -398,6 +398,11 @@ struct ext2_inode {
#define EXT2_ERRORS_PANIC 3 /* Panic */
#define EXT2_ERRORS_DEFAULT EXT2_ERRORS_CONTINUE
+/*
+ * Behaviour if use reservation window in ext2_new_blocks()
+ */
+#define EXT2_ALLOC_NORESERVE 0x1
+
/*
* Structure of the super block
*/
@@ -696,7 +701,7 @@ static inline struct ext2_inode_info *EXT2_I(struct inode *inode)
extern int ext2_bg_has_super(struct super_block *sb, int group);
extern unsigned long ext2_bg_num_gdb(struct super_block *sb, int group);
extern ext2_fsblk_t ext2_new_blocks(struct inode *, unsigned long,
- unsigned long *, int *);
+ unsigned long *, int *, unsigned int);
extern int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
unsigned int count);
extern void ext2_free_blocks (struct inode *, unsigned long,
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index acbab27fe957..462b4e873dc7 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -415,7 +415,7 @@ static int ext2_alloc_blocks(struct inode *inode,
while (1) {
count = target;
/* allocating blocks for indirect blocks and direct blocks */
- current_block = ext2_new_blocks(inode,goal,&count,err);
+ current_block = ext2_new_blocks(inode, goal, &count, err, 0);
if (*err)
goto failed_out;
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index e9546cc65db2..e076386a26f4 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -744,7 +744,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
EXT2_I(inode)->i_block_group);
unsigned long count = 1;
int block = ext2_new_blocks(inode, goal, &count,
- &error);
+ &error, 0);
if (error)
goto cleanup;
ea_idebug(inode, "creating block %d", block);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] ext2: fix race between setxattr and write back
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
2023-08-15 11:26 ` [PATCH v2 1/4] ext2: remove ext2_new_block() Ye Bin
2023-08-15 11:26 ` [PATCH v2 2/4] ext2: introduce flag argument for ext2_new_blocks() Ye Bin
@ 2023-08-15 11:26 ` Ye Bin
2023-08-15 11:26 ` [PATCH v2 4/4] ext2: dump current reservation window info Ye Bin
2023-08-16 15:53 ` [PATCH v2 0/4] ext2: fix race between setxattr and write back Jan Kara
4 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2023-08-15 11:26 UTC (permalink / raw)
To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin
There's a issue as follows:
Block Allocation Reservation Windows Map (ext2_try_to_allocate_with_rsv):
reservation window 0x000000006f105382 start: 0, end: 0
reservation window 0x000000008fd1a555 start: 1044, end: 1059
Window map complete.
kernel BUG at fs/ext2/balloc.c:1158!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
RIP: 0010:ext2_try_to_allocate_with_rsv.isra.0+0x15c4/0x1800
Call Trace:
<TASK>
ext2_new_blocks+0x935/0x1690
ext2_new_block+0x73/0xa0
ext2_xattr_set2+0x74f/0x1730
ext2_xattr_set+0x12b6/0x2260
ext2_xattr_user_set+0x9c/0x110
__vfs_setxattr+0x139/0x1d0
__vfs_setxattr_noperm+0xfc/0x370
__vfs_setxattr_locked+0x205/0x2c0
vfs_setxattr+0x19d/0x3b0
do_setxattr+0xff/0x220
setxattr+0x123/0x150
path_setxattr+0x193/0x1e0
__x64_sys_setxattr+0xc8/0x170
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Above issue may happens as follows:
setxattr write back
ext2_xattr_set
ext2_xattr_set2
ext2_new_block
ext2_new_blocks
ext2_try_to_allocate_with_rsv
alloc_new_reservation
--> group=0 [0, 1023] rsv [1016, 1023]
do_writepages
mpage_writepages
write_cache_pages
__mpage_writepage
ext2_get_block
ext2_get_blocks
ext2_alloc_branch
ext2_new_blocks
ext2_try_to_allocate_with_rsv
alloc_new_reservation
-->group=1 [1024, 2047] rsv [1044, 1059]
if ((my_rsv->rsv_start > group_last_block) ||
(my_rsv->rsv_end < group_first_block)
rsv_window_dump
BUG();
Now ext2 mkwrite delay allocate new blocks. So there maybe allocate blocks when
do write back. However, there is no concurrent protection between
ext2_xattr_set() and do_writepages().
To solve about issue don't use reservation window when allocate block for xattr.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext2/balloc.c | 15 +++++++++------
fs/ext2/xattr.c | 4 ++--
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index 99a19f2ecd6f..4ff1d831bb80 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -1247,13 +1247,16 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal,
* it's a regular file, and
* the desired window size is greater than 0 (One could use ioctl
* command EXT2_IOC_SETRSVSZ to set the window size to 0 to turn off
- * reservation on that particular file)
+ * reservation on that particular file), and unspecified Do not use
+ * reservation window.
*/
- block_i = EXT2_I(inode)->i_block_alloc_info;
- if (block_i) {
- windowsz = block_i->rsv_window_node.rsv_goal_size;
- if (windowsz > 0)
- my_rsv = &block_i->rsv_window_node;
+ if (!(flag & EXT2_ALLOC_NORESERVE)) {
+ block_i = EXT2_I(inode)->i_block_alloc_info;
+ if (block_i) {
+ windowsz = block_i->rsv_window_node.rsv_goal_size;
+ if (windowsz > 0)
+ my_rsv = &block_i->rsv_window_node;
+ }
}
if (!ext2_has_free_blocks(sbi)) {
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index e076386a26f4..21725b8d04c8 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -743,8 +743,8 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
ext2_fsblk_t goal = ext2_group_first_block_no(sb,
EXT2_I(inode)->i_block_group);
unsigned long count = 1;
- int block = ext2_new_blocks(inode, goal, &count,
- &error, 0);
+ int block = ext2_new_blocks(inode, goal, &count, &error,
+ EXT2_ALLOC_NORESERVE);
if (error)
goto cleanup;
ea_idebug(inode, "creating block %d", block);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] ext2: dump current reservation window info
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
` (2 preceding siblings ...)
2023-08-15 11:26 ` [PATCH v2 3/4] ext2: fix race between setxattr and write back Ye Bin
@ 2023-08-15 11:26 ` Ye Bin
2023-08-16 15:53 ` [PATCH v2 0/4] ext2: fix race between setxattr and write back Jan Kara
4 siblings, 0 replies; 6+ messages in thread
From: Ye Bin @ 2023-08-15 11:26 UTC (permalink / raw)
To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin
There's report BUG in 'ext2_try_to_allocate_with_rsv()', although there's
now dump of all reservation windows information. But there's unknown which
window is being processed.So this is not helpful for locating the issue.
To better analyze the problem, dump the information about reservation window
that is being processed. And just bail with error instead of BUG here.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext2/balloc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index 4ff1d831bb80..f09f413030a5 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -1131,8 +1131,13 @@ ext2_try_to_allocate_with_rsv(struct super_block *sb, unsigned int group,
if ((my_rsv->rsv_start > group_last_block) ||
(my_rsv->rsv_end < group_first_block)) {
+ ext2_error(sb, __func__,
+ "Reservation out of group %u range goal %d fsb[%lu,%lu] rsv[%lu, %lu]",
+ group, grp_goal, group_first_block,
+ group_last_block, my_rsv->rsv_start,
+ my_rsv->rsv_end);
rsv_window_dump(&EXT2_SB(sb)->s_rsv_window_root, 1);
- BUG();
+ return -1;
}
ret = ext2_try_to_allocate(sb, group, bitmap_bh, grp_goal,
&num, &my_rsv->rsv_window);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4] ext2: fix race between setxattr and write back
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
` (3 preceding siblings ...)
2023-08-15 11:26 ` [PATCH v2 4/4] ext2: dump current reservation window info Ye Bin
@ 2023-08-16 15:53 ` Jan Kara
4 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2023-08-16 15:53 UTC (permalink / raw)
To: Ye Bin; +Cc: jack, linux-ext4, linux-kernel
On Tue 15-08-23 19:26:08, Ye Bin wrote:
> Diff v2 vs v1:
> 1. Do not use reservation window when allocate block for xattr.
> 2. Remove BUG() in ext2_try_to_allocate_with_rsv().
Thanks! I did't a few smaller spelling fixups on commit but otherwise the
patches look good so I've added them to my tree.
Honza
>
> Ye Bin (4):
> ext2: remove ext2_new_block()
> ext2: introduce flag argument for ext2_new_blocks()
> ext2: fix race between setxattr and write back
> ext2: dump current reservation window info
>
> fs/ext2/balloc.c | 32 +++++++++++++++++---------------
> fs/ext2/ext2.h | 8 ++++++--
> fs/ext2/inode.c | 2 +-
> fs/ext2/xattr.c | 4 +++-
> 4 files changed, 27 insertions(+), 19 deletions(-)
>
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-16 15:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-15 11:26 [PATCH v2 0/4] ext2: fix race between setxattr and write back Ye Bin
2023-08-15 11:26 ` [PATCH v2 1/4] ext2: remove ext2_new_block() Ye Bin
2023-08-15 11:26 ` [PATCH v2 2/4] ext2: introduce flag argument for ext2_new_blocks() Ye Bin
2023-08-15 11:26 ` [PATCH v2 3/4] ext2: fix race between setxattr and write back Ye Bin
2023-08-15 11:26 ` [PATCH v2 4/4] ext2: dump current reservation window info Ye Bin
2023-08-16 15:53 ` [PATCH v2 0/4] ext2: fix race between setxattr and write back Jan Kara
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 read-only IMAP folder(s) and NNTP newsgroup(s).