Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr
@ 2020-06-09 10:53 Ritesh Harjani
  2020-06-10  6:25 ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Ritesh Harjani @ 2020-06-09 10:53 UTC (permalink / raw)
  To: linux-ext4
  Cc: jack, tytso, Hillf Danton, linux-fsdevel, Ritesh Harjani,
	Borislav Petkov, Marek Szyprowski, syzbot+82f324bb69744c5f6969

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 <bp@alien8.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
Reported-by: syzbot+82f324bb69744c5f6969@syzkaller.appspotmail.com
---
 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr
  2020-06-09 10:53 [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr Ritesh Harjani
@ 2020-06-10  6:25 ` Christoph Hellwig
  2020-06-10  6:34   ` Ritesh Harjani
  2020-06-11 15:03   ` Theodore Y. Ts'o
  0 siblings, 2 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-06-10  6:25 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: linux-ext4, jack, tytso, Hillf Danton, linux-fsdevel,
	Borislav Petkov, Marek Szyprowski, syzbot+82f324bb69744c5f6969

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 <hch@lst.de>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr
  2020-06-10  6:25 ` Christoph Hellwig
@ 2020-06-10  6:34   ` Ritesh Harjani
  2020-06-11 15:03   ` Theodore Y. Ts'o
  1 sibling, 0 replies; 5+ messages in thread
From: Ritesh Harjani @ 2020-06-10  6:34 UTC (permalink / raw)
  To: Christoph Hellwig, tytso
  Cc: linux-ext4, jack, Hillf Danton, linux-fsdevel, Borislav Petkov,
	Marek Szyprowski, syzbot+82f324bb69744c5f6969



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 <hch@lst.de>
> 

Thanks

-ritesh

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr
  2020-06-10  6:25 ` Christoph Hellwig
  2020-06-10  6:34   ` Ritesh Harjani
@ 2020-06-11 15:03   ` Theodore Y. Ts'o
  1 sibling, 0 replies; 5+ messages in thread
From: Theodore Y. Ts'o @ 2020-06-11 15:03 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ritesh Harjani, linux-ext4, jack, Hillf Danton, linux-fsdevel,
	Borislav Petkov, Marek Szyprowski, syzbot+82f324bb69744c5f6969

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 <hch@lst.de>

Thanks, applied.

						- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr
       [not found] <20200609123716.16888-1-hdanton@sina.com>
@ 2020-06-10  2:06 ` Ritesh Harjani
  0 siblings, 0 replies; 5+ messages in thread
From: Ritesh Harjani @ 2020-06-10  2:06 UTC (permalink / raw)
  To: Hillf Danton
  Cc: linux-ext4, jack, tytso, Markus Elfring, linux-kernel,
	linux-fsdevel, Borislav Petkov, Marek Szyprowski,
	syzbot+82f324bb69744c5f6969



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-11 15:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 10:53 [PATCHv2 1/1] ext4: mballoc: Use this_cpu_read instead of this_cpu_ptr Ritesh Harjani
2020-06-10  6:25 ` Christoph Hellwig
2020-06-10  6:34   ` Ritesh Harjani
2020-06-11 15:03   ` Theodore Y. Ts'o
     [not found] <20200609123716.16888-1-hdanton@sina.com>
2020-06-10  2:06 ` Ritesh Harjani

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