linux-kernel - Re: [PATCH v2] percpu-refcount: Use normal instead of RCU-sched"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Nov 2019 11:44:03 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Christopher Lameter <cl@...ux.com>
Cc:     Dennis Zhou <dennis@...nel.org>, linux-kernel@...r.kernel.org,
        Tejun Heo <tj@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH v2] percpu-refcount: Use normal instead of RCU-sched"

On 2019-11-08 18:17:47 [+0000], Christopher Lameter wrote:
> On Fri, 8 Nov 2019, Sebastian Andrzej Siewior wrote:
> 
> > diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
> > index 7aef0abc194a2..390031e816dcd 100644
> > --- a/include/linux/percpu-refcount.h
> > +++ b/include/linux/percpu-refcount.h
> > @@ -186,14 +186,14 @@ static inline void percpu_ref_get_many(struct percpu_ref *ref, unsigned long nr)
> >  {
> >  	unsigned long __percpu *percpu_count;
> >
> > -	rcu_read_lock_sched();
> > +	rcu_read_lock();
> >
> >  	if (__ref_is_percpu(ref, &percpu_count))
> >  		this_cpu_add(*percpu_count, nr);
> 
> You can use
> 
> 	__this_cpu_add()
> 
> instead since rcu_read_lock implies preempt disable.

Paul may correct me but rcu_read_lock() does not imply
preempt_disable().  rcu_read_lock() does preempt_disable() on preemption
models other than "Low-Latency Desktop". You can be preempted in a
RCU-read section.

> This will not change the code for x86 but other platforms that do not use
> atomic operation here will be able to avoid including to code to disable
> preempt for the per cpu operations.

x86 triggers this warning with the suggested change:

| BUG: using __this_cpu_add() in preemptible [00000000] code: login/2370
| caller is blk_mq_get_request+0x74/0x4c0
| CPU: 0 PID: 2370 Comm: login Not tainted 5.4.0-rc7+ #82
| Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-1 04/01/2014
| Call Trace:
|  dump_stack+0x7a/0xaa
|  __this_cpu_preempt_check.cold+0x49/0x4e
|  blk_mq_get_request+0x74/0x4c0
|  blk_mq_make_request+0x111/0x890
|  generic_make_request+0xd3/0x3f0
|  submit_bio+0x42/0x140
|  submit_bh_wbc.isra.0+0x13f/0x170
|  ll_rw_block+0xa0/0xb0
|  __breadahead+0x3f/0x70
|  __ext4_get_inode_loc+0x40a/0x520
|  __ext4_iget+0x10a/0xcf0
|  ext4_lookup+0x106/0x1f0
|  lookup_open+0x267/0x8e0
|  path_openat+0x7c8/0xcb0
|  do_filp_open+0x8c/0x100
|  do_sys_open+0x17a/0x230
|  __x64_sys_openat+0x1b/0x20
|  do_syscall_64+0x5a/0x230
|  entry_SYSCALL_64_after_hwframe+0x49/0xbe

> Same is valid for all other per cpu operations in the patch.

Sebastian