* [PATCH 0/1] lockdep: fix warning: print_lock_trace defined but not used @ 2019-05-21 7:08 john.hubbard 2019-05-21 7:08 ` [PATCH 1/1] " john.hubbard 0 siblings, 1 reply; 6+ messages in thread From: john.hubbard @ 2019-05-21 7:08 UTC (permalink / raw) To: LKML Cc: John Hubbard, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Paul E . McKenney, Peter Zijlstra, Thomas Gleixner, Will Deacon From: John Hubbard <jhubbard@nvidia.com> Hi, This is a second attempt to send this. The first attempt [1] went via mailgw.nvidia.com, and did not receive a DKIM signature, so I think it probably got discarded by many systems. This one goes via gmail, which is what I was using before. We'll keep working on getting the NVIDIA outgoing email servers working with mailing lists, but meanwhile I'm back to using "envelope" techniques, I guess. I was hoping to use a "native" email address, but not quite there yet. :) Anyway, here's the original cover letter: I ran across this minor warning while building against today's linux.git. The proposed trivial fix leaves it a little fragile from a "what if someone adds a new call to print_lock_trace()" point of view, but I believe that it makes the current combination of ifdefs accurate, anyway. [1] https://lkml.org/lkml/2019/5/19/60 Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Will Deacon <will.deacon@arm.com> John Hubbard (1): lockdep: fix warning: print_lock_trace defined but not used kernel/locking/lockdep.c | 3 +++ 1 file changed, 3 insertions(+) -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] lockdep: fix warning: print_lock_trace defined but not used 2019-05-21 7:08 [PATCH 0/1] lockdep: fix warning: print_lock_trace defined but not used john.hubbard @ 2019-05-21 7:08 ` john.hubbard 2019-06-09 13:51 ` Paul E. McKenney 0 siblings, 1 reply; 6+ messages in thread From: john.hubbard @ 2019-05-21 7:08 UTC (permalink / raw) To: LKML Cc: John Hubbard, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Paul E . McKenney, Peter Zijlstra, Thomas Gleixner, Will Deacon From: John Hubbard <jhubbard@nvidia.com> Commit 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") moved the only usage of print_lock_trace() that was originally outside of the CONFIG_PROVE_LOCKING case. It moved that usage into a different case: CONFIG_PROVE_LOCKING && CONFIG_TRACE_IRQFLAGS. That leaves things not symmetrical, and as a result, the following warning fires on my build, when I have !CONFIG_TRACE_IRQFLAGS && !CONFIG_PROVE_LOCKING set: kernel/locking/lockdep.c:2821:13: warning: ‘print_lock_trace’ defined but not used [-Wunused-function] Fix this by only defining print_lock_trace() in cases in which is it called. Fixes: 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- kernel/locking/lockdep.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index d06190fa5082..3065dc36c27a 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2817,11 +2817,14 @@ static inline int validate_chain(struct task_struct *curr, return 1; } +#if defined(CONFIG_TRACE_IRQFLAGS) static void print_lock_trace(struct lock_trace *trace, unsigned int spaces) { } #endif +#endif + /* * We are building curr_chain_key incrementally, so double-check * it from scratch, to make sure that it's done correctly: -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lockdep: fix warning: print_lock_trace defined but not used 2019-05-21 7:08 ` [PATCH 1/1] " john.hubbard @ 2019-06-09 13:51 ` Paul E. McKenney 2019-06-09 23:38 ` John Hubbard 2019-06-10 0:09 ` [PATCH v2] " john.hubbard 0 siblings, 2 replies; 6+ messages in thread From: Paul E. McKenney @ 2019-06-09 13:51 UTC (permalink / raw) To: john.hubbard Cc: LKML, John Hubbard, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, Will Deacon On Tue, May 21, 2019 at 12:08:08AM -0700, john.hubbard@gmail.com wrote: > From: John Hubbard <jhubbard@nvidia.com> > > Commit 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside > CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") moved the only usage of > print_lock_trace() that was originally outside of the CONFIG_PROVE_LOCKING > case. It moved that usage into a different case: CONFIG_PROVE_LOCKING && > CONFIG_TRACE_IRQFLAGS. That leaves things not symmetrical, and as a result, > the following warning fires on my build, when I have > > !CONFIG_TRACE_IRQFLAGS && !CONFIG_PROVE_LOCKING > > set: > > kernel/locking/lockdep.c:2821:13: warning: ‘print_lock_trace’ defined > but not used [-Wunused-function] > > Fix this by only defining print_lock_trace() in cases in which is it > called. > > Fixes: 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") > Cc: Frederic Weisbecker <frederic@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > kernel/locking/lockdep.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index d06190fa5082..3065dc36c27a 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -2817,11 +2817,14 @@ static inline int validate_chain(struct task_struct *curr, > return 1; > } > > +#if defined(CONFIG_TRACE_IRQFLAGS) > static void print_lock_trace(struct lock_trace *trace, unsigned int spaces) This works, but another approach is to put "__maybe_unused" in the above declaration, which avoids the need to have "#if" in a .c file. But this file already has quite a few #if and #ifdef commands, so maybe it is OK here. Also, "#ifdef CONFIG_TRACE_IRQFLAGS" is a bit more conventional than the above, should the "__maybe_unused" be undesirable. Yet another approach is to move this function to include/linux/lockdep.h, where #ifdef is considered less objectionable. But I must defer to the maintainers. Thanx, Paul > { > } > #endif > > +#endif > + > /* > * We are building curr_chain_key incrementally, so double-check > * it from scratch, to make sure that it's done correctly: > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lockdep: fix warning: print_lock_trace defined but not used 2019-06-09 13:51 ` Paul E. McKenney @ 2019-06-09 23:38 ` John Hubbard 2019-06-10 0:09 ` [PATCH v2] " john.hubbard 1 sibling, 0 replies; 6+ messages in thread From: John Hubbard @ 2019-06-09 23:38 UTC (permalink / raw) To: paulmck, john.hubbard Cc: LKML, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, Will Deacon On 6/9/19 6:51 AM, Paul E. McKenney wrote: > On Tue, May 21, 2019 at 12:08:08AM -0700, john.hubbard@gmail.com wrote: >> From: John Hubbard <jhubbard@nvidia.com> >> >> Commit 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside >> CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") moved the only usage of >> print_lock_trace() that was originally outside of the CONFIG_PROVE_LOCKING >> case. It moved that usage into a different case: CONFIG_PROVE_LOCKING && >> CONFIG_TRACE_IRQFLAGS. That leaves things not symmetrical, and as a result, >> the following warning fires on my build, when I have >> >> !CONFIG_TRACE_IRQFLAGS && !CONFIG_PROVE_LOCKING >> >> set: >> >> kernel/locking/lockdep.c:2821:13: warning: ‘print_lock_trace’ defined >> but not used [-Wunused-function] >> >> Fix this by only defining print_lock_trace() in cases in which is it >> called. >> >> Fixes: 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") >> Cc: Frederic Weisbecker <frederic@kernel.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Linus Torvalds <torvalds@linux-foundation.org> >> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Will Deacon <will.deacon@arm.com> >> Signed-off-by: John Hubbard <jhubbard@nvidia.com> >> --- >> kernel/locking/lockdep.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >> index d06190fa5082..3065dc36c27a 100644 >> --- a/kernel/locking/lockdep.c >> +++ b/kernel/locking/lockdep.c >> @@ -2817,11 +2817,14 @@ static inline int validate_chain(struct task_struct *curr, >> return 1; >> } >> >> +#if defined(CONFIG_TRACE_IRQFLAGS) >> static void print_lock_trace(struct lock_trace *trace, unsigned int spaces) > > This works, but another approach is to put "__maybe_unused" in the > above declaration, which avoids the need to have "#if" in a .c file. Good idea, that approach appeals to me here, because tracing is a natural fit for "might not be used" types of functions. > But this file already has quite a few #if and #ifdef commands, so maybe > it is OK here. > > Also, "#ifdef CONFIG_TRACE_IRQFLAGS" is a bit more conventional than > the above, should the "__maybe_unused" be undesirable. ah, OK, I'll keep that in mind. (The two seemed identical to my mind, but it's good to make things look like surrounding code, of course.) thanks, -- John Hubbard NVIDIA > > Yet another approach is to move this function to include/linux/lockdep.h, > where #ifdef is considered less objectionable. > > But I must defer to the maintainers. > > Thanx, Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] lockdep: fix warning: print_lock_trace defined but not used 2019-06-09 13:51 ` Paul E. McKenney 2019-06-09 23:38 ` John Hubbard @ 2019-06-10 0:09 ` john.hubbard 2019-06-18 16:00 ` Paul E. McKenney 1 sibling, 1 reply; 6+ messages in thread From: john.hubbard @ 2019-06-10 0:09 UTC (permalink / raw) To: LKML Cc: John Hubbard, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Paul E . McKenney, Peter Zijlstra, Thomas Gleixner, Will Deacon From: John Hubbard <jhubbard@nvidia.com> Commit 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") moved the only usage of print_lock_trace() that was originally outside of the CONFIG_PROVE_LOCKING case. It moved that usage into a different case: CONFIG_PROVE_LOCKING && CONFIG_TRACE_IRQFLAGS. That leaves things not symmetrical, and as a result, the following warning fires on my build, when I have !CONFIG_TRACE_IRQFLAGS && !CONFIG_PROVE_LOCKING set: kernel/locking/lockdep.c:2821:13: warning: ‘print_lock_trace’ defined but not used [-Wunused-function] Fix this by annotating print_lock_trace() with "__maybe_unused". Thanks to Paul E. McKenney for suggesting this less intrusive fix, as compared to adding more ifdef noise. Fixes: 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- kernel/locking/lockdep.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index c47788fa85f9..2726dafdb29b 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2818,7 +2818,8 @@ static inline int validate_chain(struct task_struct *curr, return 1; } -static void print_lock_trace(struct lock_trace *trace, unsigned int spaces) +static void __maybe_unused print_lock_trace(struct lock_trace *trace, + unsigned int spaces) { } #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] lockdep: fix warning: print_lock_trace defined but not used 2019-06-10 0:09 ` [PATCH v2] " john.hubbard @ 2019-06-18 16:00 ` Paul E. McKenney 0 siblings, 0 replies; 6+ messages in thread From: Paul E. McKenney @ 2019-06-18 16:00 UTC (permalink / raw) To: john.hubbard Cc: LKML, John Hubbard, Frederic Weisbecker, Andrew Morton, Linus Torvalds, Peter Zijlstra, Thomas Gleixner, Will Deacon On Sun, Jun 09, 2019 at 05:09:33PM -0700, john.hubbard@gmail.com wrote: > From: John Hubbard <jhubbard@nvidia.com> > > Commit 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside > CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") moved the only usage of > print_lock_trace() that was originally outside of the CONFIG_PROVE_LOCKING > case. It moved that usage into a different case: CONFIG_PROVE_LOCKING && > CONFIG_TRACE_IRQFLAGS. That leaves things not symmetrical, and as a result, > the following warning fires on my build, when I have > > !CONFIG_TRACE_IRQFLAGS && !CONFIG_PROVE_LOCKING > > set: > > kernel/locking/lockdep.c:2821:13: warning: ‘print_lock_trace’ defined > but not used [-Wunused-function] > > Fix this by annotating print_lock_trace() with "__maybe_unused". > Thanks to Paul E. McKenney for suggesting this less intrusive fix, > as compared to adding more ifdef noise. > > Fixes: 0d2cc3b34532 ("locking/lockdep: Move valid_state() inside CONFIG_TRACE_IRQFLAGS && CONFIG_PROVE_LOCKING") > > Cc: Frederic Weisbecker <frederic@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Tested-by: Paul E. McKenney <paulmck@linux.ibm.com> > --- > kernel/locking/lockdep.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > index c47788fa85f9..2726dafdb29b 100644 > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -2818,7 +2818,8 @@ static inline int validate_chain(struct task_struct *curr, > return 1; > } > > -static void print_lock_trace(struct lock_trace *trace, unsigned int spaces) > +static void __maybe_unused print_lock_trace(struct lock_trace *trace, > + unsigned int spaces) > { > } > #endif > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-18 16:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-21 7:08 [PATCH 0/1] lockdep: fix warning: print_lock_trace defined but not used john.hubbard 2019-05-21 7:08 ` [PATCH 1/1] " john.hubbard 2019-06-09 13:51 ` Paul E. McKenney 2019-06-09 23:38 ` John Hubbard 2019-06-10 0:09 ` [PATCH v2] " john.hubbard 2019-06-18 16:00 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox