From: Alexey Kardashevskiy <aik@ozlabs.ru> To: Nicholas Piggin <npiggin@gmail.com>, Peter Zijlstra <peterz@infradead.org> Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org> Subject: Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state Date: Fri, 24 Jul 2020 14:16:39 +1000 [thread overview] Message-ID: <1884dcea-9ecd-a1f3-21bb-213c655e2480@ozlabs.ru> (raw) In-Reply-To: <1595506730.3mvrxktem5.astroid@bobo.none> On 23/07/2020 23:11, Nicholas Piggin wrote: > Excerpts from Peter Zijlstra's message of July 23, 2020 9:40 pm: >> On Thu, Jul 23, 2020 at 08:56:14PM +1000, Nicholas Piggin wrote: >> >>> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h >>> index 3a0db7b0b46e..35060be09073 100644 >>> --- a/arch/powerpc/include/asm/hw_irq.h >>> +++ b/arch/powerpc/include/asm/hw_irq.h >>> @@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void) >>> #define powerpc_local_irq_pmu_save(flags) \ >>> do { \ >>> raw_local_irq_pmu_save(flags); \ >>> - trace_hardirqs_off(); \ >>> + if (!raw_irqs_disabled_flags(flags)) \ >>> + trace_hardirqs_off(); \ >>> } while(0) >>> #define powerpc_local_irq_pmu_restore(flags) \ >>> do { \ >>> - if (raw_irqs_disabled_flags(flags)) { \ >>> - raw_local_irq_pmu_restore(flags); \ >>> - trace_hardirqs_off(); \ >>> - } else { \ >>> + if (!raw_irqs_disabled_flags(flags)) \ >>> trace_hardirqs_on(); \ >>> - raw_local_irq_pmu_restore(flags); \ >>> - } \ >>> + raw_local_irq_pmu_restore(flags); \ >>> } while(0) >> >> You shouldn't be calling lockdep from NMI context! > > After this patch it doesn't. > > trace_hardirqs_on/off implementation appears to expect to be called in NMI > context though, for some reason. > >> That is, I recently >> added suport for that on x86: >> >> https://lkml.kernel.org/r/20200623083721.155449112@infradead.org >> https://lkml.kernel.org/r/20200623083721.216740948@infradead.org >> >> But you need to be very careful on how you order things, as you can see >> the above relies on preempt_count() already having been incremented with >> NMI_MASK. > > Hmm. My patch seems simpler. And your patches fix my error while Peter's do not: IRQs not enabled as expected WARNING: CPU: 0 PID: 1377 at /home/aik/p/kernel/kernel/softirq.c:169 __local_bh_enable_ip+0x118/0x190 > > I don't know this stuff very well, I don't really understand what your patch > enables for x86 but at least it shouldn't be incompatible with this one > AFAIKS. > > Thanks, > Nick > -- Alexey
WARNING: multiple messages have this Message-ID (diff)
From: Alexey Kardashevskiy <aik@ozlabs.ru> To: Nicholas Piggin <npiggin@gmail.com>, Peter Zijlstra <peterz@infradead.org> Cc: linux-arch@vger.kernel.org, Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state Date: Fri, 24 Jul 2020 14:16:39 +1000 [thread overview] Message-ID: <1884dcea-9ecd-a1f3-21bb-213c655e2480@ozlabs.ru> (raw) In-Reply-To: <1595506730.3mvrxktem5.astroid@bobo.none> On 23/07/2020 23:11, Nicholas Piggin wrote: > Excerpts from Peter Zijlstra's message of July 23, 2020 9:40 pm: >> On Thu, Jul 23, 2020 at 08:56:14PM +1000, Nicholas Piggin wrote: >> >>> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h >>> index 3a0db7b0b46e..35060be09073 100644 >>> --- a/arch/powerpc/include/asm/hw_irq.h >>> +++ b/arch/powerpc/include/asm/hw_irq.h >>> @@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void) >>> #define powerpc_local_irq_pmu_save(flags) \ >>> do { \ >>> raw_local_irq_pmu_save(flags); \ >>> - trace_hardirqs_off(); \ >>> + if (!raw_irqs_disabled_flags(flags)) \ >>> + trace_hardirqs_off(); \ >>> } while(0) >>> #define powerpc_local_irq_pmu_restore(flags) \ >>> do { \ >>> - if (raw_irqs_disabled_flags(flags)) { \ >>> - raw_local_irq_pmu_restore(flags); \ >>> - trace_hardirqs_off(); \ >>> - } else { \ >>> + if (!raw_irqs_disabled_flags(flags)) \ >>> trace_hardirqs_on(); \ >>> - raw_local_irq_pmu_restore(flags); \ >>> - } \ >>> + raw_local_irq_pmu_restore(flags); \ >>> } while(0) >> >> You shouldn't be calling lockdep from NMI context! > > After this patch it doesn't. > > trace_hardirqs_on/off implementation appears to expect to be called in NMI > context though, for some reason. > >> That is, I recently >> added suport for that on x86: >> >> https://lkml.kernel.org/r/20200623083721.155449112@infradead.org >> https://lkml.kernel.org/r/20200623083721.216740948@infradead.org >> >> But you need to be very careful on how you order things, as you can see >> the above relies on preempt_count() already having been incremented with >> NMI_MASK. > > Hmm. My patch seems simpler. And your patches fix my error while Peter's do not: IRQs not enabled as expected WARNING: CPU: 0 PID: 1377 at /home/aik/p/kernel/kernel/softirq.c:169 __local_bh_enable_ip+0x118/0x190 > > I don't know this stuff very well, I don't really understand what your patch > enables for x86 but at least it shouldn't be incompatible with this one > AFAIKS. > > Thanks, > Nick > -- Alexey
next prev parent reply other threads:[~2020-07-24 4:16 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-23 10:56 [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state Nicholas Piggin 2020-07-23 10:56 ` Nicholas Piggin 2020-07-23 10:56 ` [PATCH 2/2] lockdep: warn on redundant or incorrect irq state changes Nicholas Piggin 2020-07-23 10:56 ` Nicholas Piggin 2020-07-24 2:57 ` kernel test robot 2020-07-24 2:57 ` kernel test robot 2020-07-24 2:57 ` kernel test robot 2020-07-24 2:57 ` kernel test robot 2020-07-24 2:57 ` kernel test robot 2020-08-04 10:00 ` Alexey Kardashevskiy 2020-08-04 10:00 ` Alexey Kardashevskiy 2020-07-23 11:40 ` [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state Peter Zijlstra 2020-07-23 11:40 ` Peter Zijlstra 2020-07-23 13:11 ` Nicholas Piggin 2020-07-23 13:11 ` Nicholas Piggin 2020-07-23 14:59 ` Peter Zijlstra 2020-07-23 14:59 ` Peter Zijlstra 2020-07-23 16:20 ` Nicholas Piggin 2020-07-23 16:20 ` Nicholas Piggin 2020-07-24 4:16 ` Alexey Kardashevskiy [this message] 2020-07-24 4:16 ` Alexey Kardashevskiy 2020-07-24 5:59 ` Nicholas Piggin 2020-07-24 5:59 ` Nicholas Piggin 2020-07-26 7:40 ` Alexey Kardashevskiy 2020-07-26 7:40 ` Alexey Kardashevskiy 2020-07-24 6:16 ` Athira Rajeev 2020-07-24 6:16 ` Athira Rajeev 2020-07-24 2:19 ` kernel test robot 2020-07-24 2:19 ` kernel test robot 2020-07-24 2:19 ` kernel test robot 2020-07-24 2:19 ` kernel test robot 2020-07-24 2:19 ` kernel test robot 2020-07-24 3:15 ` kernel test robot 2020-07-24 3:15 ` kernel test robot 2020-07-24 3:15 ` kernel test robot 2020-07-24 3:15 ` kernel test robot 2020-07-24 3:15 ` kernel test robot 2020-07-25 20:26 ` Peter Zijlstra 2020-07-25 20:26 ` Peter Zijlstra 2020-07-26 4:14 ` Nicholas Piggin 2020-07-26 4:14 ` Nicholas Piggin 2020-07-26 11:59 ` peterz 2020-07-26 11:59 ` peterz 2020-07-26 12:11 ` peterz 2020-07-26 12:11 ` peterz 2020-07-28 11:22 ` Nicholas Piggin 2020-07-28 11:22 ` Nicholas Piggin 2020-08-07 11:11 ` peterz 2020-08-07 11:11 ` peterz 2020-08-12 8:18 ` Nicholas Piggin 2020-08-12 8:18 ` Nicholas Piggin 2020-08-12 10:35 ` peterz 2020-08-12 10:35 ` peterz 2020-08-18 7:22 ` Nicholas Piggin 2020-08-18 7:22 ` Nicholas Piggin 2020-08-18 15:41 ` peterz 2020-08-18 15:41 ` peterz 2020-08-18 23:54 ` Nicholas Piggin 2020-08-18 23:54 ` Nicholas Piggin 2020-08-19 10:39 ` Alexey Kardashevskiy 2020-08-19 10:39 ` Alexey Kardashevskiy 2020-08-19 15:32 ` peterz 2020-08-19 15:32 ` peterz 2020-08-19 15:39 ` peterz 2020-08-19 15:39 ` peterz 2020-08-27 7:54 ` [tip: locking/core] lockdep: Only trace IRQ edges tip-bot2 for Nicholas Piggin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1884dcea-9ecd-a1f3-21bb-213c655e2480@ozlabs.ru \ --to=aik@ozlabs.ru \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mingo@redhat.com \ --cc=npiggin@gmail.com \ --cc=peterz@infradead.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.