Messages in this thread | | | Date | Fri, 11 Jun 2021 09:00:34 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH next v2 1/2] dump_stack: move cpu lock to printk.c |
| |
On Thu 2021-06-10 15:26:15, John Ogness wrote: > On 2021-06-08, Petr Mladek <pmladek@suse.com> wrote: > >> lib/dump_stack.c: In function 'dump_stack_lvl': > >> >> lib/dump_stack.c:107:2: warning: 'lock_flag' is used uninitialized in this function [-Wuninitialized] > >> 107 | printk_cpu_unlock_irqrestore(lock_flag, irq_flags); > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Interesting. I am curious that it does not complain also about > > irq_flags. But it is possible the it reports only the first problem. > > Strangely enough, if I set a value for @lock_flag, it is happy and does > not complain about @irq_flags. Probably a compiler oversight.
Yeah, it is strange.
> > Anyway, we will likely need to do some trickery via #define to tell > > the compiler that the value is set. > > This is on ARCH=mips and !CONFIG_SMP. So the value is _not_ getting > set. (The static inline function does nothing.) > > By changing printk_cpu_unlock_irqrestore() to use pointers: > > static inline void printk_cpu_unlock_irqrestore(bool *lock_flag, unsigned long *irq_flags) > > then the warning disappears. Indeed, by not using pointers on unlock, > technically data is copied that was never initialized. I thought maybe > the compiler would optimize all that out, but it seems that it does not. > > I have no problems using pointers for unlock(). It was strange using > pointers for lock(), but not for unlock() anyway. > > Or would you prefer something else?
I would actually prefer to introduce the macros and pass the flags without referencing.
I was about to write that I did not mind. But then it came to me that it might be worth being compatible with the other irqsafe()/irqrestore() APIs. It seems that people are pretty used to pass flags directly:
$> git grep irqsave.*flags | wc -l 17084 $> git grep irqsave.*\&flags | wc -l 15
That said, I do not resist on it. It will not block the patchset if you decided to used the pointers. The lock should not be used widely...
Best Regards, Petr
| |