Re: [PATCH] sched: use rq_lock/unlock in online_fair_sched_group

[Phil Auld <pauld@redhat.com>]

On Tue, Aug 06, 2019 at 03:03:34PM +0200 Peter Zijlstra wrote:
> On Thu, Aug 01, 2019 at 09:37:49AM -0400, Phil Auld wrote:
> > Enabling WARN_DOUBLE_CLOCK in /sys/kernel/debug/sched_features causes
> 
> ISTR there were more issues; but it sure is good to start picking them
> off.
> 

Following up on this I hit another in rt.c which looks like:

[  156.348854] Call Trace:
[  156.351301]  <IRQ>
[  156.353322]  sched_rt_period_timer+0x124/0x350
[  156.357766]  ? sched_rt_rq_enqueue+0x90/0x90
[  156.362037]  __hrtimer_run_queues+0xfb/0x270
[  156.366303]  hrtimer_interrupt+0x122/0x270
[  156.370403]  smp_apic_timer_interrupt+0x6a/0x140
[  156.375022]  apic_timer_interrupt+0xf/0x20
[  156.379119]  </IRQ>

It looks like the same issue of not using the rq_lock* wrappers and
hence not using the pinning. From looking at the code there is at 
least one potential hit in deadline.c in the push_dl_task path with 
find_lock_later_rq but I have not hit that in practice.

This commit, which introduced the warning, seems to imply that the use
of the rq_lock* wrappers is required, at least for any sections that will
call update_rq_clock:

commit 26ae58d23b94a075ae724fd18783a3773131cfbc
Author: Peter Zijlstra <peterz@infradead.org>
Date:   Mon Oct 3 16:53:49 2016 +0200

    sched/core: Add WARNING for multiple update_rq_clock() calls
    
    Now that we have no missing calls, add a warning to find multiple
    calls.
    
    By having only a single update_rq_clock() call per rq-lock section,
    the section appears 'atomic' wrt time.


Is that the case? Otherwise we have these false positives.

I can spin up patches if so. 


Thanks,
Phil


--