Re: linux-next: build failure after merge of the printk tree
On Wed, Jul 29, 2020 at 09:03:11PM +1000, Stephen Rothwell wrote: > > After merging the printk tree, today's linux-next build (powerpc > allyesconfig) failed like this: Hi Stephen: This loop was introduced recently by the powerpc tree with https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=aa65ff6b18e0366db1790609956a4ac7308c5668 powerpc/64s: Implement queued spinlocks and rwlocks However the loop itself goes back further and in fact someone has already tried to work around it by adding ifdefs on CONFIG_PARAVIRT in asm-generic/qspinlock_types.h. I'll try to fix this properly. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
Hi Sergey, On Tue, 28 Jul 2020 10:54:08 +0900 Sergey Senozhatsky wrote: > >> Sorry about that. This will be fixed for tomorrow's build. The problems > is that we have commit dependency between printk and locking tree. So I > pulled the > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/core&id=c935cd62d3fe985d7f0ebea185d2759e8992e96f > > into printk/for-next. OK, thanks. -- Cheers, Stephen Rothwell pgpj52mUHzoC1.pgp Description: OpenPGP digital signature
Re: linux-next: build failure after merge of the printk tree
Hi Herbert, On Tue, 28 Jul 2020 11:51:19 +1000 Herbert Xu wrote: > > This patch depends on two patches in the tips tree. I presume > this build test was done without the tips tree, right? Of course it was ... Each tree merged into linux-next should really be standalone (in case e.g. Linus does not take the other tree). So to do what you want, you need those other 2 commits to be in a separate branch that both the tip tree and the printk tree can merge. -- Cheers, Stephen Rothwell pgpgDjFEE0Bjy.pgp Description: OpenPGP digital signature
Re: linux-next: build failure after merge of the printk tree
On (20/07/28 11:49), Stephen Rothwell wrote:
[..]
> Caused by commit
>
> b4a461e72bcb ("printk: Make linux/printk.h self-contained")
>
> This *may* be interacting with other include file changes in linux-next.
>
> I have reverted that commit for today.
Hi Stephen,
Sorry about that. This will be fixed for tomorrow's build. The problems
is that we have commit dependency between printk and locking tree. So I
pulled the
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/core&id=c935cd62d3fe985d7f0ebea185d2759e8992e96f
into printk/for-next.
-ss
Re: linux-next: build failure after merge of the printk tree
On Tue, Jul 28, 2020 at 11:49:27AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> In file included from include/linux/printk.h:10,
> from include/linux/kernel.h:15,
> from include/linux/list.h:9,
> from include/linux/lockdep.h:43,
> from include/linux/spinlock_types.h:18,
> from include/linux/genalloc.h:32,
> from drivers/soc/fsl/qe/qe_common.c:16:
> include/linux/ratelimit_types.h:16:2: error: unknown type name
> 'raw_spinlock_t'
>16 | raw_spinlock_t lock; /* protect the state */
> | ^~
> In file included from include/linux/wait.h:9,
> from include/linux/pid.h:6,
> from include/linux/sched.h:14,
> from include/linux/ratelimit.h:6,
> from include/linux/dev_printk.h:16,
> from include/linux/device.h:15,
> from include/linux/node.h:18,
> from include/linux/cpu.h:17,
> from include/linux/of_device.h:5,
> from drivers/soc/fsl/qe/qe_common.c:19:
> include/linux/ratelimit.h: In function 'ratelimit_state_init':
> include/linux/ratelimit.h:14:21: error: passing argument 1 of
> '__raw_spin_lock_init' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>14 | raw_spin_lock_init(&rs->lock);
> include/linux/spinlock.h:102:24: note: in definition of macro
> 'raw_spin_lock_init'
> 102 | __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
> |^~~~
> include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka
> 'struct raw_spinlock *'} but argument is of type 'int *'
>95 | extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char
> *name,
> |^~~~
>
> Caused by commit
>
> b4a461e72bcb ("printk: Make linux/printk.h self-contained")
>
> This *may* be interacting with other include file changes in linux-next.
>
> I have reverted that commit for today.
This patch depends on two patches in the tips tree. I presume
this build test was done without the tips tree, right?
Cheers,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
On Tue 2020-06-23 17:17:38, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote: > > > PS: And yes, it makes sense to push both patches via a single tree to > > make sure that the lockdep.h split is done first. > > That's what I got you tip/locking/header for, pull that topic branch > into your tree. I see, I misunderstood it. I am going to merge the branch once I get the updated 2nd patch. Best Regards, Petr
Re: linux-next: build failure after merge of the printk tree
On Wed, Jun 24, 2020 at 10:19:29AM +0200, Petr Mladek wrote: > On Wed 2020-06-24 10:20:58, Herbert Xu wrote: > > On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote: > > > > > > It is similar cycle: > > > > > > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> > > > spinlock_types.h > > > > > > But this time it happens via list.h -> kernel.h ->printk.h. > > > Where list.h needs READ_ONCE() stuff from compiler.h. > > > > But this is exactly the loop that's fixed by the lockdep_types > > patch. > > I think that this problem is similar to the one on s390 that was > solved by compiler.h in arch/s390/include/asm/bug.h > > It is related to > https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local > > It seems that we need the same change also in list.h. Oh I see. I misunderstood what you said in the first email. > Do you mean to repost the 2nd patch with the change in > include/linux/list.h? OK let me test this and repost. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
On Wed 2020-06-24 10:20:58, Herbert Xu wrote: > On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote: > > > > It is similar cycle: > > > > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h > > > > But this time it happens via list.h -> kernel.h ->printk.h. > > Where list.h needs READ_ONCE() stuff from compiler.h. > > But this is exactly the loop that's fixed by the lockdep_types > patch. I think that this problem is similar to the one on s390 that was solved by compiler.h in arch/s390/include/asm/bug.h It is related to https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local It seems that we need the same change also in list.h. > Did you get a compile failure with *just* the lockdep > patch? No, I see the problem only with both patches. > > PS: And yes, it makes sense to push both patches via a single tree to > > make sure that the lockdep.h split is done first. > > OK, can I repost this patch with your ack then? Do you mean to repost the 2nd patch with the change in include/linux/list.h? This would be great. Best Regards, Petr
Re: linux-next: build failure after merge of the printk tree
On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote: > > It is similar cycle: > > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h > > But this time it happens via list.h -> kernel.h ->printk.h. > Where list.h needs READ_ONCE() stuff from compiler.h. But this is exactly the loop that's fixed by the lockdep_types patch. Did you get a compile failure with *just* the lockdep patch? > PS: And yes, it makes sense to push both patches via a single tree to > make sure that the lockdep.h split is done first. OK, can I repost this patch with your ack then? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote: > PS: And yes, it makes sense to push both patches via a single tree to > make sure that the lockdep.h split is done first. That's what I got you tip/locking/header for, pull that topic branch into your tree.
Re: linux-next: build failure after merge of the printk tree
On Tue 2020-06-23 22:19:37, Herbert Xu wrote: > On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote: > > > > I have removed the problematic commit for now. It tried to remove > > some cyclic dependencies from heavily used include files. It clearly > > needs more love. > > Hmm, the cyclic dependencies are there because you didn't pull in > the lockdep_types patch. The printk patch must go on top of the > lockdep_types patch. How about just putting this into the x86 tree > alongside the lockdep_types patch? I see the problem with both patches. > In file included from include/linux/printk.h:10, > from include/linux/kernel.h:15, > from include/linux/list.h:9, > from include/linux/lockdep.h:43, > from include/linux/spinlock_types.h:18, > from include/linux/genalloc.h:32, > from drivers/soc/fsl/qe/qe_common.c:16: > include/linux/ratelimit_types.h:16:2: error: unknown type name > 'raw_spinlock_t' >16 | raw_spinlock_t lock; /* protect the state */ It is similar cycle: spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h But this time it happens via list.h -> kernel.h ->printk.h. Where list.h needs READ_ONCE() stuff from compiler.h. My "allmodconfig" build has successfully finished with the following extra fix on top of the two patches: diff --git a/include/linux/list.h b/include/linux/list.h index aff44d34f4e4..6d606c4036ce 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include /* * Simple doubly linked list implementation. I suggest to bundle this into the 2nd patch that makes linux/printk.h self-contained. Best Regards, Petr PS: And yes, it makes sense to push both patches via a single tree to make sure that the lockdep.h split is done first.
Re: linux-next: build failure after merge of the printk tree
On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote: > > I have removed the problematic commit for now. It tried to remove > some cyclic dependencies from heavily used include files. It clearly > needs more love. Hmm, the cyclic dependencies are there because you didn't pull in the lockdep_types patch. The printk patch must go on top of the lockdep_types patch. How about just putting this into the x86 tree alongside the lockdep_types patch? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
On Tue 2020-06-23 10:26:55, Stephen Rothwell wrote:
> Hi Stephen,
>
> On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell
> wrote:
> >
> > Hi all,
> >
> > After merging the printk tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > In file included from include/linux/printk.h:10,
> > from include/linux/kernel.h:15,
> > from include/linux/list.h:9,
> > from include/linux/lockdep.h:43,
> > from include/linux/spinlock_types.h:18,
> > from include/linux/genalloc.h:32,
> > from drivers/soc/fsl/qe/qe_common.c:16:
> > include/linux/ratelimit_types.h:16:2: error: unknown type name
> > 'raw_spinlock_t'
> >16 | raw_spinlock_t lock; /* protect the state */
> > | ^~
> > In file included from include/linux/wait.h:9,
> > from include/linux/pid.h:6,
> > from include/linux/sched.h:14,
> > from include/linux/ratelimit.h:6,
> > from include/linux/dev_printk.h:16,
> > from include/linux/device.h:15,
> > from include/linux/node.h:18,
> > from include/linux/cpu.h:17,
> > from include/linux/of_device.h:5,
> > from drivers/soc/fsl/qe/qe_common.c:19:
> > include/linux/ratelimit.h: In function 'ratelimit_state_init':
> > include/linux/ratelimit.h:14:21: error: passing argument 1 of
> > '__raw_spin_lock_init' from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> >14 | raw_spin_lock_init(&rs->lock);
> > include/linux/spinlock.h:102:24: note: in definition of macro
> > 'raw_spin_lock_init'
> > 102 | __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
> > |^~~~
> > include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka
> > 'struct raw_spinlock *'} but argument is of type 'int *'
> >95 | extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char
> > *name,
> > |^~~~
> >
> > Caused by commit
> >
> > 494c8512c90e ("printk: Make linux/printk.h self-contained")
> >
> > changing include files is hadrer than it loooks :-(
> >
> > I have used the printk tree from next-20200618 for today.
>
> I am still getting this failure.
I have removed the problematic commit for now. It tried to remove
some cyclic dependencies from heavily used include files. It clearly
needs more love.
I am sorry for the late reaction. I have semi-lost mails from last 4 days.
I am still trying to recover them.
Best Regards,
Petr
Re: linux-next: build failure after merge of the printk tree
Hi Stephen,
On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell
wrote:
>
> Hi all,
>
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> In file included from include/linux/printk.h:10,
> from include/linux/kernel.h:15,
> from include/linux/list.h:9,
> from include/linux/lockdep.h:43,
> from include/linux/spinlock_types.h:18,
> from include/linux/genalloc.h:32,
> from drivers/soc/fsl/qe/qe_common.c:16:
> include/linux/ratelimit_types.h:16:2: error: unknown type name
> 'raw_spinlock_t'
>16 | raw_spinlock_t lock; /* protect the state */
> | ^~
> In file included from include/linux/wait.h:9,
> from include/linux/pid.h:6,
> from include/linux/sched.h:14,
> from include/linux/ratelimit.h:6,
> from include/linux/dev_printk.h:16,
> from include/linux/device.h:15,
> from include/linux/node.h:18,
> from include/linux/cpu.h:17,
> from include/linux/of_device.h:5,
> from drivers/soc/fsl/qe/qe_common.c:19:
> include/linux/ratelimit.h: In function 'ratelimit_state_init':
> include/linux/ratelimit.h:14:21: error: passing argument 1 of
> '__raw_spin_lock_init' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>14 | raw_spin_lock_init(&rs->lock);
> include/linux/spinlock.h:102:24: note: in definition of macro
> 'raw_spin_lock_init'
> 102 | __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
> |^~~~
> include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka
> 'struct raw_spinlock *'} but argument is of type 'int *'
>95 | extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char
> *name,
> |^~~~
>
> Caused by commit
>
> 494c8512c90e ("printk: Make linux/printk.h self-contained")
>
> changing include files is hadrer than it loooks :-(
>
> I have used the printk tree from next-20200618 for today.
I am still getting this failure.
--
Cheers,
Stephen Rothwell
pgpfW9sZKHnnU.pgp
Description: OpenPGP digital signature
Re: linux-next: build failure after merge of the printk tree
On Sun, Jun 21, 2020 at 01:15:54PM +1000, Stephen Rothwell wrote:
>
> Caused by commit
>
> 494c8512c90e ("printk: Make linux/printk.h self-contained")
>
> changing include files is hadrer than it loooks :-(
That's because this patch depends on the lockdep_types patch which
is in the x86 tree. Petr, this patch needs to go on top of the
lockdep_types patch.
Thanks,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: linux-next: build failure after merge of the printk tree
On Sat 2018-03-03 23:47:39, Sergey Senozhatsky wrote: > Cc-ing Tejun > > On (03/02/18 16:54), Petr Mladek wrote: > [..] > > > (Though it is not immediately obvious why.) > > > > It is a mistery to me. The error appears when I move any of > > dump_stack_print_info() or show_regs_print_info() function > > definitions from kernel/printk/printk.c to lib/dump_stack.c. > > All the other changes seems unrelated. > > > > The thing is that we basically do not touch dump_stack() definition > > by that patch. > > Apparently dump_stack_print_info() was in lib/dump_stack.c a long > time ago, but it was deliberately moved to printk.c, when kernel gained > a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin, > define their own dump_stack() symbol and make it global via EXPORT_SYMBOL. > > In case of blackfin that arch-specific dump_stack() symbol invokes a > global dump_stack_print_info(). If we move dump_stack_print_info() back > to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o > and lib/dump_stack.o, which results in multiple definitions error. > If we move dump_stack_print_info() out on libdump_stack.o, then we > never link with lib/dump_stack.o Ah, I have finally understood the meaning of the libs-y kbuild variable. It is a nice source of these strange build failures. > ... so what are we going to do with that. > > a) we can drop the patch and cherry pick only the kexec part > > b) we can try to mark dummy lib/dump_stack() as __weak >EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific >definitions. Using the weak symbol makes perfect sense. I am going to look at the patch. Thanks a lot everybody for help. Best Regards, Petr
Re: linux-next: build failure after merge of the printk tree
On (03/05/18 13:27), Greentime Hu wrote: [..] > >> Opinions? Will this work? > > > > I would think b) is better, thanks for the fix! > > > Hi, > > b works in nds32. > Thanks for the fix :) Greentime, Dave, thanks! I'll send out a patch then. Petr, once the patch has enough Ack/etc do you want to pick it up? We need to merge it first, before Dave's patch. -ss
Re: linux-next: build failure after merge of the printk tree
2018-03-05 11:20 GMT+08:00 Dave Young :
> On 03/03/18 at 11:47pm, Sergey Senozhatsky wrote:
>> Cc-ing Tejun
>>
>> On (03/02/18 16:54), Petr Mladek wrote:
>> [..]
>> > > (Though it is not immediately obvious why.)
>> >
>> > It is a mistery to me. The error appears when I move any of
>> > dump_stack_print_info() or show_regs_print_info() function
>> > definitions from kernel/printk/printk.c to lib/dump_stack.c.
>> > All the other changes seems unrelated.
>> >
>> > The thing is that we basically do not touch dump_stack() definition
>> > by that patch.
>>
>> Apparently dump_stack_print_info() was in lib/dump_stack.c a long
>> time ago, but it was deliberately moved to printk.c, when kernel gained
>> a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
>> define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
>>
>> In case of blackfin that arch-specific dump_stack() symbol invokes a
>> global dump_stack_print_info(). If we move dump_stack_print_info() back
>> to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
>> and lib/dump_stack.o, which results in multiple definitions error.
>> If we move dump_stack_print_info() out on libdump_stack.o, then we
>> never link with lib/dump_stack.o
>>
>> ... so what are we going to do with that.
>>
>> a) we can drop the patch and cherry pick only the kexec part
>>
>> b) we can try to mark dummy lib/dump_stack() as __weak
>>EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
>>definitions.
>>
>>So we will end up with EXPORT_SYMBOL dump_stack() and archs
>>may re-define it. If some arch will accidentally mark its
>>own dump_stack() as EXPORT_SYMBOL then there should be a
>>linkage warning - a symbol is exported twice.
>>
>>
>> Something like below.
>>
>> Opinions? Will this work?
>
> I would think b) is better, thanks for the fix!
>
Hi,
b works in nds32.
Thanks for the fix :)
>>
>>
>> = 8< =
>>
>> From: Sergey Senozhatsky
>> Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak
>>
>> ---
>> arch/blackfin/kernel/dumpstack.c | 1 -
>> arch/nds32/kernel/traps.c| 2 --
>> lib/dump_stack.c | 4 ++--
>> 3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/blackfin/kernel/dumpstack.c
>> b/arch/blackfin/kernel/dumpstack.c
>> index 3c992c1f8ef2..61af017130cd 100644
>> --- a/arch/blackfin/kernel/dumpstack.c
>> +++ b/arch/blackfin/kernel/dumpstack.c
>> @@ -174,4 +174,3 @@ void dump_stack(void)
>> show_stack(current, &stack);
>> trace_buffer_restore(tflags);
>> }
>> -EXPORT_SYMBOL(dump_stack);
>> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
>> index 8828b4aeb72b..455bb0787367 100644
>> --- a/arch/nds32/kernel/traps.c
>> +++ b/arch/nds32/kernel/traps.c
>> @@ -166,8 +166,6 @@ void dump_stack(void)
>> __dump(NULL, base_reg);
>> }
>>
>> -EXPORT_SYMBOL(dump_stack);
>> -
>> void show_stack(struct task_struct *tsk, unsigned long *sp)
>> {
>> unsigned long *base_reg;
>> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
>> index 5cff72f18c4a..9cf4465dbffa 100644
>> --- a/lib/dump_stack.c
>> +++ b/lib/dump_stack.c
>> @@ -85,7 +85,7 @@ static void __dump_stack(void)
>> #ifdef CONFIG_SMP
>> static atomic_t dump_lock = ATOMIC_INIT(-1);
>>
>> -asmlinkage __visible void dump_stack(void)
>> +asmlinkage __weak __visible void dump_stack(void)
>> {
>> unsigned long flags;
>> int was_locked;
>> @@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
>> local_irq_restore(flags);
>> }
>> #else
>> -asmlinkage __visible void dump_stack(void)
>> +asmlinkage __weak __visible void dump_stack(void)
>> {
>> __dump_stack();
>> }
>> --
>> 2.16.2
>>
>
> Thanks
> Dave
Re: linux-next: build failure after merge of the printk tree
On 03/03/18 at 11:47pm, Sergey Senozhatsky wrote:
> Cc-ing Tejun
>
> On (03/02/18 16:54), Petr Mladek wrote:
> [..]
> > > (Though it is not immediately obvious why.)
> >
> > It is a mistery to me. The error appears when I move any of
> > dump_stack_print_info() or show_regs_print_info() function
> > definitions from kernel/printk/printk.c to lib/dump_stack.c.
> > All the other changes seems unrelated.
> >
> > The thing is that we basically do not touch dump_stack() definition
> > by that patch.
>
> Apparently dump_stack_print_info() was in lib/dump_stack.c a long
> time ago, but it was deliberately moved to printk.c, when kernel gained
> a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
> define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
>
> In case of blackfin that arch-specific dump_stack() symbol invokes a
> global dump_stack_print_info(). If we move dump_stack_print_info() back
> to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
> and lib/dump_stack.o, which results in multiple definitions error.
> If we move dump_stack_print_info() out on libdump_stack.o, then we
> never link with lib/dump_stack.o
>
> ... so what are we going to do with that.
>
> a) we can drop the patch and cherry pick only the kexec part
>
> b) we can try to mark dummy lib/dump_stack() as __weak
>EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
>definitions.
>
>So we will end up with EXPORT_SYMBOL dump_stack() and archs
>may re-define it. If some arch will accidentally mark its
>own dump_stack() as EXPORT_SYMBOL then there should be a
>linkage warning - a symbol is exported twice.
>
>
> Something like below.
>
> Opinions? Will this work?
I would think b) is better, thanks for the fix!
>
>
> = 8< =
>
> From: Sergey Senozhatsky
> Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak
>
> ---
> arch/blackfin/kernel/dumpstack.c | 1 -
> arch/nds32/kernel/traps.c| 2 --
> lib/dump_stack.c | 4 ++--
> 3 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/blackfin/kernel/dumpstack.c
> b/arch/blackfin/kernel/dumpstack.c
> index 3c992c1f8ef2..61af017130cd 100644
> --- a/arch/blackfin/kernel/dumpstack.c
> +++ b/arch/blackfin/kernel/dumpstack.c
> @@ -174,4 +174,3 @@ void dump_stack(void)
> show_stack(current, &stack);
> trace_buffer_restore(tflags);
> }
> -EXPORT_SYMBOL(dump_stack);
> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
> index 8828b4aeb72b..455bb0787367 100644
> --- a/arch/nds32/kernel/traps.c
> +++ b/arch/nds32/kernel/traps.c
> @@ -166,8 +166,6 @@ void dump_stack(void)
> __dump(NULL, base_reg);
> }
>
> -EXPORT_SYMBOL(dump_stack);
> -
> void show_stack(struct task_struct *tsk, unsigned long *sp)
> {
> unsigned long *base_reg;
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index 5cff72f18c4a..9cf4465dbffa 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -85,7 +85,7 @@ static void __dump_stack(void)
> #ifdef CONFIG_SMP
> static atomic_t dump_lock = ATOMIC_INIT(-1);
>
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __weak __visible void dump_stack(void)
> {
> unsigned long flags;
> int was_locked;
> @@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
> local_irq_restore(flags);
> }
> #else
> -asmlinkage __visible void dump_stack(void)
> +asmlinkage __weak __visible void dump_stack(void)
> {
> __dump_stack();
> }
> --
> 2.16.2
>
Thanks
Dave
Re: linux-next: build failure after merge of the printk tree
Cc-ing Tejun
On (03/02/18 16:54), Petr Mladek wrote:
[..]
> > (Though it is not immediately obvious why.)
>
> It is a mistery to me. The error appears when I move any of
> dump_stack_print_info() or show_regs_print_info() function
> definitions from kernel/printk/printk.c to lib/dump_stack.c.
> All the other changes seems unrelated.
>
> The thing is that we basically do not touch dump_stack() definition
> by that patch.
Apparently dump_stack_print_info() was in lib/dump_stack.c a long
time ago, but it was deliberately moved to printk.c, when kernel gained
a "generic" (dummy) dump_stack() fallback. Some archs, like blackfin,
define their own dump_stack() symbol and make it global via EXPORT_SYMBOL.
In case of blackfin that arch-specific dump_stack() symbol invokes a
global dump_stack_print_info(). If we move dump_stack_print_info() back
to lib/dump_stack.c then we link both with arch/blackfin/dumpstack.o
and lib/dump_stack.o, which results in multiple definitions error.
If we move dump_stack_print_info() out on libdump_stack.o, then we
never link with lib/dump_stack.o
... so what are we going to do with that.
a) we can drop the patch and cherry pick only the kexec part
b) we can try to mark dummy lib/dump_stack() as __weak
EXPORT_SYMBOL and remove EXPORT_SYMBOL from arch-specific
definitions.
So we will end up with EXPORT_SYMBOL dump_stack() and archs
may re-define it. If some arch will accidentally mark its
own dump_stack() as EXPORT_SYMBOL then there should be a
linkage warning - a symbol is exported twice.
Something like below.
Opinions? Will this work?
= 8< =
From: Sergey Senozhatsky
Subject: [PATCH] dump_stack: mark dummy dump_stack() as weak
---
arch/blackfin/kernel/dumpstack.c | 1 -
arch/nds32/kernel/traps.c| 2 --
lib/dump_stack.c | 4 ++--
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
index 3c992c1f8ef2..61af017130cd 100644
--- a/arch/blackfin/kernel/dumpstack.c
+++ b/arch/blackfin/kernel/dumpstack.c
@@ -174,4 +174,3 @@ void dump_stack(void)
show_stack(current, &stack);
trace_buffer_restore(tflags);
}
-EXPORT_SYMBOL(dump_stack);
diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
index 8828b4aeb72b..455bb0787367 100644
--- a/arch/nds32/kernel/traps.c
+++ b/arch/nds32/kernel/traps.c
@@ -166,8 +166,6 @@ void dump_stack(void)
__dump(NULL, base_reg);
}
-EXPORT_SYMBOL(dump_stack);
-
void show_stack(struct task_struct *tsk, unsigned long *sp)
{
unsigned long *base_reg;
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 5cff72f18c4a..9cf4465dbffa 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -85,7 +85,7 @@ static void __dump_stack(void)
#ifdef CONFIG_SMP
static atomic_t dump_lock = ATOMIC_INIT(-1);
-asmlinkage __visible void dump_stack(void)
+asmlinkage __weak __visible void dump_stack(void)
{
unsigned long flags;
int was_locked;
@@ -118,7 +118,7 @@ asmlinkage __visible void dump_stack(void)
local_irq_restore(flags);
}
#else
-asmlinkage __visible void dump_stack(void)
+asmlinkage __weak __visible void dump_stack(void)
{
__dump_stack();
}
--
2.16.2
Re: linux-next: build failure after merge of the printk tree
Hi Petr, On Fri, 2 Mar 2018 16:54:54 +0100 Petr Mladek wrote: > > It is a mistery to me. The error appears when I move any of > dump_stack_print_info() or show_regs_print_info() function > definitions from kernel/printk/printk.c to lib/dump_stack.c. > All the other changes seems unrelated. Presumably because the Blackfin dumpstack() calls dump_stack_print_info() so if you move that into lib/dumpstack.o, then that file is dragged in and it contains another copy of dumpstack(). -- Cheers, Stephen Rothwell pgphUeZI4Wzps.pgp Description: OpenPGP digital signature
Re: linux-next: build failure after merge of the printk tree
On Fri 2018-03-02 16:07:32, Stephen Rothwell wrote:
> Hi Petr,
>
> After merging the printk tree, today's linux-next build (bfin
> BF518F-EZBRD_defconfig) failed like this:
>
> lib/dump_stack.o: In function `dump_stack':
> lib/dump_stack.c:122: multiple definition of `dump_stack'
> arch/blackfin/kernel/dumpstack.o:arch/blackfin/kernel/dumpstack.c:166: first
> defined here
>
> Presumably caused by commit
>
> 8040af489957 ("printk: move dump stack related code to lib/dump_stack.c")
I could confirm that it is caused by this commit. I have temporary
removed it from printk.git.
> (Though it is not immediately obvious why.)
It is a mistery to me. The error appears when I move any of
dump_stack_print_info() or show_regs_print_info() function
definitions from kernel/printk/printk.c to lib/dump_stack.c.
All the other changes seems unrelated.
The thing is that we basically do not touch dump_stack() definition
by that patch.
> This fails all the blackfin builds. nds32 (a new architecture) also
> has a dump_stack function.
Good to know!
Best Regards,
Petr
Re: linux-next: build failure after merge of the printk tree
On Thu 2017-12-07 10:26:31, Sergey Senozhatsky wrote:
> On (12/07/17 11:37), Stephen Rothwell wrote:
> [..]
> > include/linux/kallsyms.h: In function 'dereference_symbol_descriptor':
> > include/linux/kallsyms.h:63:8: error: implicit declaration of function
> > '__module_address' [-Werror=implicit-function-declaration]
> > mod = __module_address((unsigned long)ptr);
> > ^
> > include/linux/kallsyms.h:67:9: error: implicit declaration of function
> > 'dereference_module_function_descriptor'
> > [-Werror=implicit-function-declaration]
> >ptr = dereference_module_function_descriptor(mod, ptr);
> > ^
> >
> > and many more ...
> >
> > Caused by commit
> >
> > 78675fe41d57 ("symbol lookup: introduce dereference_symbol_descriptor()")
> >
> > I have used the printk tree from next-20171206 (with its version of the
> > above patch reverted due to yesterday's error) for today.
>
> will fix :(
Sigh, the dependencies are quite complicated here.
Anyway, Sergey's fix for these non-x86 architectures is merged in printk.git
now.
Best Regards,
Petr
Re: linux-next: build failure after merge of the printk tree
On (12/07/17 11:37), Stephen Rothwell wrote:
[..]
> include/linux/kallsyms.h: In function 'dereference_symbol_descriptor':
> include/linux/kallsyms.h:63:8: error: implicit declaration of function
> '__module_address' [-Werror=implicit-function-declaration]
> mod = __module_address((unsigned long)ptr);
> ^
> include/linux/kallsyms.h:67:9: error: implicit declaration of function
> 'dereference_module_function_descriptor'
> [-Werror=implicit-function-declaration]
>ptr = dereference_module_function_descriptor(mod, ptr);
> ^
>
> and many more ...
>
> Caused by commit
>
> 78675fe41d57 ("symbol lookup: introduce dereference_symbol_descriptor()")
>
> I have used the printk tree from next-20171206 (with its version of the
> above patch reverted due to yesterday's error) for today.
will fix :(
-ss
Re: linux-next: build failure after merge of the printk tree
On (12/06/17 11:17), Petr Mladek wrote:
> On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> > Hi Petr,
> >
> > After merging the printk tree, today's linux-next build (x86_64
> > allnoconfig) failed like this:
> >
> > lib/vsprintf.o: In function `pointer':
> > vsprintf.c:(.text+0x2260): undefined reference to
> > `dereference_symbol_descriptor'
> >
> > Caused by commit
> >
> > b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
>
> I have just pushed the fixed commit from Sergey and will improve my
> test coverage.
>
> > I have reverted that commit for today.
>
> It should work now. I am sorry for the troubles.
Stephen,
please keep that commit
("symbol lookup: introduce dereference_symbol_descriptor()")
reverted in today's [20171207] linux-next. we have one more problem
with it.
-ss
Re: linux-next: build failure after merge of the printk tree
On (12/06/17 11:17), Petr Mladek wrote:
> On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> > Hi Petr,
> >
> > After merging the printk tree, today's linux-next build (x86_64
> > allnoconfig) failed like this:
> >
> > lib/vsprintf.o: In function `pointer':
> > vsprintf.c:(.text+0x2260): undefined reference to
> > `dereference_symbol_descriptor'
> >
> > Caused by commit
> >
> > b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
>
> I have just pushed the fixed commit from Sergey and will improve my
> test coverage.
>
> > I have reverted that commit for today.
>
> It should work now. I am sorry for the troubles.
it was completely my fault. sorry about that.
-ss
Re: linux-next: build failure after merge of the printk tree
On Wed 2017-12-06 14:23:00, Stephen Rothwell wrote:
> Hi Petr,
>
> After merging the printk tree, today's linux-next build (x86_64
> allnoconfig) failed like this:
>
> lib/vsprintf.o: In function `pointer':
> vsprintf.c:(.text+0x2260): undefined reference to
> `dereference_symbol_descriptor'
>
> Caused by commit
>
> b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
I have just pushed the fixed commit from Sergey and will improve my
test coverage.
> I have reverted that commit for today.
It should work now. I am sorry for the troubles.
Best Regards,
Petr
Re: linux-next: build failure after merge of the printk tree
Hello,
On (12/06/17 14:23), Stephen Rothwell wrote:
> Hi Petr,
>
> After merging the printk tree, today's linux-next build (x86_64
> allnoconfig) failed like this:
>
> lib/vsprintf.o: In function `pointer':
> vsprintf.c:(.text+0x2260): undefined reference to
> `dereference_symbol_descriptor'
>
> Caused by commit
>
> b77929215209 ("symbol lookup: introduce dereference_symbol_descriptor()")
>
> I have reverted that commit for today.
thanks. will fix it.
-ss
