lkml.org 
[lkml]   [2018]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] cgroup: use irqsave in cgroup_rstat_flush_locked()
On 2018-07-03 23:35:39 [+0200], To Tejun Heo wrote:
> On 2018-07-03 13:24:24 [-0700], Tejun Heo wrote:
> > (cc'ing Peter and Ingo for lockdep)
> >
> > Hello, Sebastian.
> Hi Tejun,
>
> > On Tue, Jul 03, 2018 at 06:45:44PM +0200, Sebastian Andrzej Siewior wrote:
> > > All callers of cgroup_rstat_flush_locked() acquire cgroup_rstat_lock
> > > either with spin_lock_irq() or spin_lock_irqsave().
> >
> > So, irq is always disabled in cgroup_rstat_flush_locked().
>
> on not RT enabled kernels. On RT enabled kernels spin_lock_irq.*() is
> turned into a sleeping spinlock which do not disable interrupts.
>
> > > cgroup_rstat_flush_locked() itself acquires cgroup_rstat_cpu_lock which
> > > is a raw_spin_lock. This lock is also acquired in cgroup_rstat_updated()
> > > in IRQ context and therefore requires _irqsave() locking suffix in
> > > cgroup_rstat_flush_locked().
> >
> > Yes, the cpu locks should be irqsafe too; however, as irq is always
> > disabled in that function, save/restore is redundant, no?
>
> as I pointed out above only the raw_spin_lock_t really disables
> interrupts on -RT. That is the difference between those two.
>
> > > Since there is no difference between spin_lock_t and raw_spin_lock_t
> > > on !RT lockdep does not complain here. On RT lockdep complains because
> > > the interrupts were not disabled here and a deadlock is possible.
> >
> > We at least used to do this in the kernel - manipulating irqsafe locks
> > with spin_lock/unlock() if the irq state is known, whether enabled or
> > disabled, and ISTR lockdep being smart enough to track actual irq
> > state to determine irq safety. Am I misremembering or is this
> > different on RT kernels?
>
> No, this is correct. So on !RT kernels the spin_lock_irq() disables
> interrupts and the raw_spin_lock() has the interrupts already disabled,
> everything is good. On RT kernels the spin_lock_irq() does not disable
> interrupts and the raw_spin_lock() acquires the lock with enabled
> interrupts and lockdep complains properly.
> lockdep sees the hardirq path via:
>
> {IN-HARDIRQ-W} state was registered at:
> lock_acquire+0x9e/0x250
> _raw_spin_lock_irqsave+0x38/0x50
> cgroup_rstat_updated+0x57/0x100
> cgroup_base_stat_cputime_account_end.isra.6+0x17/0x60
> __cgroup_account_cputime_field+0x49/0x60
> account_system_index_time+0xdb/0x1f0
> account_system_time+0x3f/0x70
> account_process_tick+0x59/0x80
> update_process_times+0x1d/0x50
> tick_sched_handle+0x20/0x60
> tick_sched_timer+0x37/0x80
> __hrtimer_run_queues+0x12c/0x6d0
> hrtimer_interrupt+0xed/0x240
> smp_apic_timer_interrupt+0x89/0x3c0
> apic_timer_interrupt+0xf/0x20
> pin_current_cpu+0xa/0x120
> migrate_disable+0x9a/0x200
> rt_spin_lock+0x1d/0x60
> put_unused_fd+0x2c/0x50
> do_sys_open+0x23a/0x250
> __x64_sys_openat+0x1b/0x20
> do_syscall_64+0x50/0x190
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> > Thanks.

ping.

Sebastian

\
 
 \ /
  Last update: 2018-07-11 13:06    [W:0.040 / U:1.396 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site