2020-06-09 10:58:37

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr

Simplify reading a seq variable by directly using this_cpu_read API
instead of doing this_cpu_ptr and then dereferencing it.

This also avoid the below kernel BUG: which happens when
CONFIG_DEBUG_PREEMPT is enabled

BUG: using smp_processor_id() in preemptible [00000000] code: syz-fuzzer/6927
caller is ext4_mb_new_blocks+0xa4d/0x3b70 fs/ext4/mballoc.c:4711
CPU: 1 PID: 6927 Comm: syz-fuzzer Not tainted 5.7.0-next-20200602-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x18f/0x20d lib/dump_stack.c:118
check_preemption_disabled+0x20d/0x220 lib/smp_processor_id.c:48
ext4_mb_new_blocks+0xa4d/0x3b70 fs/ext4/mballoc.c:4711
ext4_ext_map_blocks+0x201b/0x33e0 fs/ext4/extents.c:4244
ext4_map_blocks+0x4cb/0x1640 fs/ext4/inode.c:626
ext4_getblk+0xad/0x520 fs/ext4/inode.c:833
ext4_bread+0x7c/0x380 fs/ext4/inode.c:883
ext4_append+0x153/0x360 fs/ext4/namei.c:67
ext4_init_new_dir fs/ext4/namei.c:2757 [inline]
ext4_mkdir+0x5e0/0xdf0 fs/ext4/namei.c:2802
vfs_mkdir+0x419/0x690 fs/namei.c:3632
do_mkdirat+0x21e/0x280 fs/namei.c:3655
do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:359
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: 42f56b7a4a7d ("ext4: mballoc: introduce pcpu seqcnt for freeing PA
to improve ENOSPC handling")
Suggested-by: Borislav Petkov <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
Reported-by: [email protected]
---
fs/ext4/mballoc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index a9083113a8c0..c0a331e2feb0 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4708,7 +4708,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
}

ac->ac_op = EXT4_MB_HISTORY_PREALLOC;
- seq = *this_cpu_ptr(&discard_pa_seq);
+ seq = this_cpu_read(discard_pa_seq);
if (!ext4_mb_use_preallocated(ac)) {
ac->ac_op = EXT4_MB_HISTORY_ALLOC;
ext4_mb_normalize_request(ac, ar);
--
2.25.4


2020-06-10 02:07:59

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr



On 6/9/20 6:07 PM, Hillf Danton wrote:
>
> On Tue, 9 Jun 2020 18:53:23 +0800 Ritesh Harjani wrote:
>>
>> Simplify reading a seq variable by directly using this_cpu_read API
>> instead of doing this_cpu_ptr and then dereferencing it.
>
> Two of the quick questions
> 1) Why can blocks discarded in a ext4 FS help allocators in another?

I am not sure if I understand your Q correctly. But here is a brief
about the patchset. If there were PA blocks just or about to be
discarded by another thread, then the current thread who is doing block
allocation should not fail with ENOSPC error instead should be able to
allocate those blocks from another thread. The concept is better
explained in the commit msgs, if more details are required.
Without this patchset (in some heavy multi-threaded use case) allocation
was failing when the overall filesystem space available was more then 50%.

>
> 2) Why is a percpu seqcount prefered over what <linux/seqlock.h>
> can offer?
>

Since this could be a multi-threaded use case, per cpu variable helps in
avoid cache line bouncing problem, which could happen when the same
variable is updated by multiple threads on different cpus.

-ritesh

2020-06-10 06:26:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr

On Tue, Jun 09, 2020 at 04:23:10PM +0530, Ritesh Harjani wrote:
> Simplify reading a seq variable by directly using this_cpu_read API
> instead of doing this_cpu_ptr and then dereferencing it.
>
> This also avoid the below kernel BUG: which happens when
> CONFIG_DEBUG_PREEMPT is enabled

I see this warning all the time with ext4 using tests VMs, so lets get
this fixed ASAP before -rc1:

Reviewed-by: Christoph Hellwig <[email protected]>

2020-06-10 06:35:49

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr



On 6/10/20 11:55 AM, Christoph Hellwig wrote:
> On Tue, Jun 09, 2020 at 04:23:10PM +0530, Ritesh Harjani wrote:
>> Simplify reading a seq variable by directly using this_cpu_read API
>> instead of doing this_cpu_ptr and then dereferencing it.
>>
>> This also avoid the below kernel BUG: which happens when
>> CONFIG_DEBUG_PREEMPT is enabled
>
> I see this warning all the time with ext4 using tests VMs, so lets get
> this fixed ASAP before -rc1:

Couldn't agree more.

>
> Reviewed-by: Christoph Hellwig <[email protected]>
>

Thanks

-ritesh

2020-06-11 15:08:15

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr

On Tue, Jun 09, 2020 at 11:25:38PM -0700, Christoph Hellwig wrote:
> On Tue, Jun 09, 2020 at 04:23:10PM +0530, Ritesh Harjani wrote:
> > Simplify reading a seq variable by directly using this_cpu_read API
> > instead of doing this_cpu_ptr and then dereferencing it.
> >
> > This also avoid the below kernel BUG: which happens when
> > CONFIG_DEBUG_PREEMPT is enabled
>
> I see this warning all the time with ext4 using tests VMs, so lets get
> this fixed ASAP before -rc1:
>
> Reviewed-by: Christoph Hellwig <[email protected]>

Thanks, applied.

- Ted