Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1645319 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-05-19 10:20 +0200
    Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-05-19 12:40 +0200
      RE: [PATCH v6 05/15] lockdep: Implement crossrelease feature "Byungchul Park" <byungchul.park@lge.com> - 2017-05-19 13:00 +0200

#1645319 — Re: [PATCH v6 05/15] lockdep: Implement crossrelease feature

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.

Thank you.
Byungchul

[toc] | [next] | [standalone]

#1645504

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

You're asking this in relation to the rollback thing, right? I think we
should only save the state when hardirq_context goes from 0->1 and
restore on 1->0.

If you're asking this for another reason, please clarify.

[toc] | [prev] | [next] | [standalone]

#1645514

> -----Original Message-----
> From: Peter Zijlstra [mailto:peterz@infradead.org]
> Sent: Friday, May 19, 2017 7:30 PM
> To: Byungchul Park
> Cc: mingo@kernel.org; tglx@linutronix.de; walken@google.com;
> boqun.feng@gmail.com; kirill@shutemov.name; linux-kernel@vger.kernel.org;
> linux-mm@kvack.org; iamjoonsoo.kim@lge.com; akpm@linux-foundation.org;
> willy@infradead.org; npiggin@gmail.com; kernel-team@lge.com
> 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web