Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-12 Thread Dave Young
On 02/09/18 at 06:15pm, Sergey Senozhatsky wrote:
> On (02/09/18 17:00), Dave Young wrote:
> [..]
> > >
> > > I'm hesitating to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.
> 
> Agreed.
> 
> > Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
> > but not sure if there are some historic reason this is not done before,
> > will do some testing see if it works.
> > 
> 
> Thanks.
> 
> Was thinking about the same thing - dump_stack() without CONFIG_PRINTK
> doesn't make that much sense anyway. Well, maybe it does in some weird
> case... Need to check. But it seems that we probably can just make the
> dependency, which already exists, explicit.

Did some testing on x86_64 and s390, I did not find any breakage, sent
the V2 just now.

> 
>   -ss

Thanks
Dave


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Sergey Senozhatsky
On (02/09/18 17:00), Dave Young wrote:
[..]
> >
> > I'm hesitating to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

Agreed.

> Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
> but not sure if there are some historic reason this is not done before,
> will do some testing see if it works.
> 

Thanks.

Was thinking about the same thing - dump_stack() without CONFIG_PRINTK
doesn't make that much sense anyway. Well, maybe it does in some weird
case... Need to check. But it seems that we probably can just make the
dependency, which already exists, explicit.

-ss


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Dave Young
On 02/09/18 at 04:51pm, Dave Young wrote:
> On 02/09/18 at 05:42pm, Sergey Senozhatsky wrote:
> > On (02/09/18 16:27), Dave Young wrote:
> > > > Seems that those functions are still defined in printk header.
> > > > Did you test !CONFIG_PRINTK build?
> > 
> > Apparently dump_stack(void) is also in printk.h
> > 
> > extern asmlinkage void dump_stack(void) __cold;
> > 
> > so it's "OK" to keep those functions in printk.h, I guess. I thought
> > that dump_stack() had its own header file...
> 
> It has not unfortunately..  The build failed because we have dummy
> functions in printk.h and redefined in lib/dump_stack.c.  I'm hesitating
> to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

Maybe conditionally build dump_stack.o only when CONFIG_PRINTK is true,
but not sure if there are some historic reason this is not done before,
will do some testing see if it works.

> 
> > 
> > > !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> > > the build, doing it now to double confirm..
> > 
> > Not sure. Please test.
> > 
> > -ss
> 
> Thanks
> Dave


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Dave Young
On 02/09/18 at 05:42pm, Sergey Senozhatsky wrote:
> On (02/09/18 16:27), Dave Young wrote:
> > > Seems that those functions are still defined in printk header.
> > > Did you test !CONFIG_PRINTK build?
> 
> Apparently dump_stack(void) is also in printk.h
> 
> extern asmlinkage void dump_stack(void) __cold;
> 
> so it's "OK" to keep those functions in printk.h, I guess. I thought
> that dump_stack() had its own header file...

It has not unfortunately..  The build failed because we have dummy
functions in printk.h and redefined in lib/dump_stack.c.  I'm hesitating
to add #ifdef CONFIG_PRINTK in lib/dump_stack.c.

> 
> > !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> > the build, doing it now to double confirm..
> 
> Not sure. Please test.
> 
>   -ss

Thanks
Dave


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Sergey Senozhatsky
On (02/09/18 16:27), Dave Young wrote:
> > Seems that those functions are still defined in printk header.
> > Did you test !CONFIG_PRINTK build?

Apparently dump_stack(void) is also in printk.h

extern asmlinkage void dump_stack(void) __cold;

so it's "OK" to keep those functions in printk.h, I guess. I thought
that dump_stack() had its own header file...

> !CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
> the build, doing it now to double confirm..

Not sure. Please test.

-ss


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Dave Young
On 02/09/18 at 05:16pm, Sergey Senozhatsky wrote:
> Hi,
> 
> On (02/09/18 16:06), Dave Young wrote:
> [..]
> > +void __init dump_stack_set_arch_desc(const char *fmt, ...)
> ..
> > +void dump_stack_print_info(const char *log_lvl)
> ..
> > +void show_regs_print_info(const char *log_lvl)
> ..
> 
> Seems that those functions are still defined in printk header.
> Did you test !CONFIG_PRINTK build?

Hmm, indeed need update printk.h for !PRINTK, thanks for catching it.
Will refresh the patch soon..

Thanks!

> 
>   -ss


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Dave Young
On 02/09/18 at 05:16pm, Sergey Senozhatsky wrote:
> Hi,
> 
> On (02/09/18 16:06), Dave Young wrote:
> [..]
> > +void __init dump_stack_set_arch_desc(const char *fmt, ...)
> ..
> > +void dump_stack_print_info(const char *log_lvl)
> ..
> > +void show_regs_print_info(const char *log_lvl)
> ..
> 
> Seems that those functions are still defined in printk header.
> Did you test !CONFIG_PRINTK build?

!CONFIG_PRINTK will use the dummy functions in printk.h, I did not test
the build, doing it now to double confirm..

Thanks
Dave

> 
>   -ss


Re: [PATCH] printk: move dump stack related code to lib/dump_stack.c

2018-02-09 Thread Sergey Senozhatsky
Hi,

On (02/09/18 16:06), Dave Young wrote:
[..]
> +void __init dump_stack_set_arch_desc(const char *fmt, ...)
..
> +void dump_stack_print_info(const char *log_lvl)
..
> +void show_regs_print_info(const char *log_lvl)
..

Seems that those functions are still defined in printk header.
Did you test !CONFIG_PRINTK build?

-ss