RE: [PATCH v6 05/15] lockdep: Implement crossrelease feature

[Byungchul Park]

> -----Original Message-----
> From: Peter Zijlstra [mailto:peterz@xxxxxxxxxxxxx]
> Sent: Friday, May 19, 2017 7:30 PM
> To: Byungchul Park
> Cc: mingo@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; walken@xxxxxxxxxx;
> boqun.feng@xxxxxxxxx; kirill@xxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-mm@xxxxxxxxx; iamjoonsoo.kim@xxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> willy@xxxxxxxxxxxxx; npiggin@xxxxxxxxx; kernel-team@xxxxxxx
> Subject: Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature
> 
> On Fri, May 19, 2017 at 05:07:08PM +0900, Byungchul Park wrote:
> > On Tue, Mar 14, 2017 at 05:18:52PM +0900, Byungchul Park wrote:
> > > Lockdep is a runtime locking correctness validator that detects and
> > > reports a deadlock or its possibility by checking dependencies between
> > > locks. It's useful since it does not report just an actual deadlock
> but
> > > also the possibility of a deadlock that has not actually happened yet.
> > > That enables problems to be fixed before they affect real systems.
> > >
> > > However, this facility is only applicable to typical locks, such as
> > > spinlocks and mutexes, which are normally released within the context
> in
> > > which they were acquired. However, synchronization primitives like
> page
> > > locks or completions, which are allowed to be released in any context,
> > > also create dependencies and can cause a deadlock. So lockdep should
> > > track these locks to do a better job. The 'crossrelease'
> implementation
> > > makes these primitives also be tracked.
> >
> > Excuse me but I have a question...
> >
> > Only for maskable irq, can I assume that hardirq are prevented within
> > hardirq context? I remember that nested interrupts were allowed in the
> > past but not recommanded. But what about now? I'm curious about the
> > overall direction of kernel and current status. It would be very
> > appriciated if you answer it.
> 
> So you're right. In general enabling IRQs from hardirq context is
> discouraged but allowed. However, if you were to do that with a lock
> held that would instantly make lockdep report a deadlock, as the lock is
> then both used from IRQ context and has IRQs enabled.
> 
> So from a locking perspective you can assume no nesting, but from a
> state tracking pov we have to deal with the nesting I think (although it
> is very rare).

Got it. Thank you.

> You're asking this in relation to the rollback thing, right? I think we

Exactly. I wanted to make it clear when implementing the rollback for
irqs and works of workqueue.

> should only save the state when hardirq_context goes from 0->1 and
> restore on 1->0.

Yes, it's already done in v6, as you are saying.

Thank you very much.