Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 11 Nov 2019 05:38:10 -0800
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Christopher Lameter <cl@linux.com>,
        Dennis Zhou <dennis@kernel.org>, linux-kernel@vger.kernel.org,
        Tejun Heo <tj@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v2] percpu-refcount: Use normal instead of RCU-sched"
Message-ID: <20191111133810.GL2865@paulmck-ThinkPad-P72>
Reply-To: paulmck@kernel.org
References: <20191002112252.ro7wpdylqlrsbamc@linutronix.de>
 <20191107091319.6zf5tmdi54amtann@linutronix.de>
 <20191107161749.GA93945@dennisz-mbp>
 <20191107162842.2qgd3db2cjmmsxeh@linutronix.de>
 <20191107165519.GA99408@dennisz-mbp>
 <20191107172434.ylz4hyxw4rbmhre2@linutronix.de>
 <20191107173653.GA1242@dennisz-mbp>
 <20191108173553.lxsdic6wa4y3ifsr@linutronix.de>
 <alpine.DEB.2.21.1911081813330.1687@www.lameter.com>
 <20191111104403.5gna4pyg25v4mdin@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191111104403.5gna4pyg25v4mdin@linutronix.de>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Nov 11, 2019 at 11:44:03AM +0100, Sebastian Andrzej Siewior wrote:
> 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.

Sebastian is quite correct, rcu_read_lock() definitely does not imply
preempt_disable() when CONFIG_PREEMPT=y.  So the splat below is expected
behavior, and not just on x86.

							Thanx, Paul

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