lkml.org 
[lkml]   [2020]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [patch 14/19] softirq: Make softirq control and processing RT aware
Date
On Fri, Nov 20 2020 at 01:26, Frederic Weisbecker wrote:
> On Fri, Nov 13, 2020 at 03:02:21PM +0100, Thomas Gleixner wrote:
>> +void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
>> +{
>> + unsigned long flags;
>> + int newcnt;
>> +
>> + WARN_ON_ONCE(in_hardirq());
>> +
>> + /* First entry of a task into a BH disabled section? */
>> + if (!current->softirq_disable_cnt) {
>> + if (preemptible()) {
>> + local_lock(&softirq_ctrl.lock);
>> + rcu_read_lock();
>
> Ah you lock RCU because local_bh_disable() implies it and
> since it doesn't disable preemption anymore, you must do it
> explicitly?
>
> Perhaps local_lock() should itself imply rcu_read_lock() ?

It's really only required for local_bh_disable(). Lemme add a comment.

>> + } else {
>> + DEBUG_LOCKS_WARN_ON(this_cpu_read(softirq_ctrl.cnt));
>> + }
>> + }
>> +
>> + preempt_disable();
>
> Do you really need to disable preemption here? Migration is disabled by local_lock()
> and I can't figure out a scenario where the below can conflict with a
> preempting task.

Indeed it's pointless.

>> + /*
>> + * Track the per CPU softirq disabled state. On RT this is per CPU
>> + * state to allow preemption of bottom half disabled sections.
>> + */
>> + newcnt = this_cpu_add_return(softirq_ctrl.cnt, cnt);
>
> __this_cpu_add_return() ?

Yep.

Thanks,

tglx

\
 
 \ /
  Last update: 2020-11-20 14:29    [W:0.218 / U:0.540 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site