linux-next: build failure after merge of the kgdb tree

linux-next: build failure after merge of the kgdb tree

[Stephen Rothwell]

Hi Jason,

After merging the kgdb tree, today's linux-next build (x86_64
allmodconfig) failed like this:

kernel/debug/kdb/kdb_main.c: In function 'kdb_sr':
kernel/debug/kdb/kdb_main.c:1822: error: '__sysrq_enabled' undeclared (first use in this function)

Caused by commit ab28988df9667f8dd70c7434c8ca463beb00b300 ("kdb: core for
kgdb back end (1 of 2)") from the kgdb tree interacting with commit
edc6e384f9d96e54a6d03068d3b03fa5b56b6449 ("") fromInput: implement SysRq
as a separate input handler the input tree.

I added the following patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 23 Mar 2010 14:45:22 +1100
Subject: [PATCH] kgdb: update for sysrq handling change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/debug/kdb/kdb_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index f1f9309..bad8e63 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1819,10 +1819,8 @@ static int kdb_sr(int argc, const char **argv)
 {
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	if (!__sysrq_enabled) {
-		kdb_printf("Auto activating sysrq\n");
-		__sysrq_enabled = 1;
-	}
+
+	sysrq_toggle_support(1);
 
 	kdb_trap_printk++;
 	handle_sysrq(*argv[1], NULL);
-- 
1.7.0.2

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Re: linux-next: build failure after merge of the kgdb tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

Hi Jason, Dmitry,

On Tue, 23 Mar 2010 15:04:43 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the kgdb tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> kernel/debug/kdb/kdb_main.c: In function 'kdb_sr':
> kernel/debug/kdb/kdb_main.c:1822: error: '__sysrq_enabled' undeclared (first use in this function)
> 
> Caused by commit ab28988df9667f8dd70c7434c8ca463beb00b300 ("kdb: core for
> kgdb back end (1 of 2)") from the kgdb tree interacting with commit
> edc6e384f9d96e54a6d03068d3b03fa5b56b6449 ("") fromInput: implement SysRq
> as a separate input handler the input tree.

I got this again today (I stopped reverting the input tree commits that I
was reverting for another build problem).  The patch below still seems to
work OK.

> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 23 Mar 2010 14:45:22 +1100
> Subject: [PATCH] kgdb: update for sysrq handling change
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  kernel/debug/kdb/kdb_main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index f1f9309..bad8e63 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1819,10 +1819,8 @@ static int kdb_sr(int argc, const char **argv)
>  {
>  	if (argc != 1)
>  		return KDB_ARGCOUNT;
> -	if (!__sysrq_enabled) {
> -		kdb_printf("Auto activating sysrq\n");
> -		__sysrq_enabled = 1;
> -	}
> +
> +	sysrq_toggle_support(1);
>  
>  	kdb_trap_printk++;
>  	handle_sysrq(*argv[1], NULL);

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: build failure after merge of the kgdb tree

[Dmitry Torokhov]

On Wednesday 24 March 2010 09:43:07 pm Stephen Rothwell wrote:
> Hi Jason, Dmitry,
> 
> On Tue, 23 Mar 2010 15:04:43 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 
wrote:
> > After merging the kgdb tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > kernel/debug/kdb/kdb_main.c: In function 'kdb_sr':
> > kernel/debug/kdb/kdb_main.c:1822: error: '__sysrq_enabled' undeclared
> > (first use in this function)
> > 
> > Caused by commit ab28988df9667f8dd70c7434c8ca463beb00b300 ("kdb: core for
> > kgdb back end (1 of 2)") from the kgdb tree interacting with commit
> > edc6e384f9d96e54a6d03068d3b03fa5b56b6449 ("") fromInput: implement SysRq
> > as a separate input handler the input tree.
> 
> I got this again today (I stopped reverting the input tree commits that I
> was reverting for another build problem).  The patch below still seems to
> work OK.

Yes, I  of __sysrq_enabled is not global anymore...

I am not sure I like kgdb enabling sysrq functonality on its own, I'd rather
export __handle_sysrq that does not check sysrq state...

> 
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Tue, 23 Mar 2010 14:45:22 +1100
> > Subject: [PATCH] kgdb: update for sysrq handling change
> > 
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > ---
> > 
> >  kernel/debug/kdb/kdb_main.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index f1f9309..bad8e63 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -1819,10 +1819,8 @@ static int kdb_sr(int argc, const char **argv)
> > 
> >  {
> >  
> >  	if (argc != 1)
> >  	
> >  		return KDB_ARGCOUNT;
> > 
> > -	if (!__sysrq_enabled) {
> > -		kdb_printf("Auto activating sysrq\n");
> > -		__sysrq_enabled = 1;
> > -	}
> > +
> > +	sysrq_toggle_support(1);
> > 
> >  	kdb_trap_printk++;
> >  	handle_sysrq(*argv[1], NULL);

Thanks.

-- 
Dmitry

Re: linux-next: build failure after merge of the kgdb tree

[Stephen Rothwell]

Hi Jason,

On Wed, 24 Mar 2010 21:59:25 -0700 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Wednesday 24 March 2010 09:43:07 pm Stephen Rothwell wrote:
> > Hi Jason, Dmitry,
> > 
> > On Tue, 23 Mar 2010 15:04:43 +1100 Stephen Rothwell <sfr@canb.auug.org.au> 
> wrote:
> > > After merging the kgdb tree, today's linux-next build (x86_64
> > > allmodconfig) failed like this:
> > > 
> > > kernel/debug/kdb/kdb_main.c: In function 'kdb_sr':
> > > kernel/debug/kdb/kdb_main.c:1822: error: '__sysrq_enabled' undeclared
> > > (first use in this function)
> > > 
> > > Caused by commit ab28988df9667f8dd70c7434c8ca463beb00b300 ("kdb: core for
> > > kgdb back end (1 of 2)") from the kgdb tree interacting with commit
> > > edc6e384f9d96e54a6d03068d3b03fa5b56b6449 ("") fromInput: implement SysRq
> > > as a separate input handler the input tree.
> > 
> > I got this again today (I stopped reverting the input tree commits that I
> > was reverting for another build problem).  The patch below still seems to
> > work OK.
> 
> Yes, I  of __sysrq_enabled is not global anymore...
> 
> I am not sure I like kgdb enabling sysrq functonality on its own, I'd rather
> export __handle_sysrq that does not check sysrq state...

The input changes that require change in the kgdb tree are now in Linus'
tree (I think) so this patch (or something else) should be added to the
kgdb tree before it is sent to Linus for merging.

Dmitry, any thoughts?

Here is the patch again:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 23 Mar 2010 14:45:22 +1100
Subject: [PATCH] kgdb: update for sysrq handling change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/debug/kdb/kdb_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index f1f9309..bad8e63 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1819,10 +1819,8 @@ static int kdb_sr(int argc, const char **argv)
 {
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	if (!__sysrq_enabled) {
-		kdb_printf("Auto activating sysrq\n");
-		__sysrq_enabled = 1;
-	}
+
+	sysrq_toggle_support(1);
 
 	kdb_trap_printk++;
 	handle_sysrq(*argv[1], NULL);
-- 
1.7.0.2


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: linux-next: build failure after merge of the kgdb tree

[Jason Wessel]

On 05/20/2010 07:21 PM, Stephen Rothwell wrote:
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  kernel/debug/kdb/kdb_main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index f1f9309..bad8e63 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1819,10 +1819,8 @@ static int kdb_sr(int argc, const char **argv)
>  {
>  	if (argc != 1)
>  		return KDB_ARGCOUNT;
> -	if (!__sysrq_enabled) {
> -		kdb_printf("Auto activating sysrq\n");
> -		__sysrq_enabled = 1;
> -	}
> +
> +	sysrq_toggle_support(1);
>  
>  	kdb_trap_printk++;
>  	handle_sysrq(*argv[1], NULL);
>   


Before brute force toggling it, it seems we should check the value and
restore it after the execution of handle_sysrq().

I'll have to look and see if there is an access function for this.

Jason.

Re: linux-next: build failure after merge of the kgdb tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Hi Jason,

On Thu, 20 May 2010 19:49:51 -0500 Jason Wessel <jason.wessel@windriver.com> wrote:
>
> Before brute force toggling it, it seems we should check the value and
> restore it after the execution of handle_sysrq().

Indeed, at the time I couldn't find an easy way to do that.

> I'll have to look and see if there is an access function for this.

Great, thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: build failure after merge of the kgdb tree

[Dmitry Torokhov]

On Fri, May 21, 2010 at 11:09:38AM +1000, Stephen Rothwell wrote:
> Hi Jason,
> 
> On Thu, 20 May 2010 19:49:51 -0500 Jason Wessel <jason.wessel@windriver.com> wrote:
> >
> > Before brute force toggling it, it seems we should check the value and
> > restore it after the execution of handle_sysrq().
> 
> Indeed, at the time I couldn't find an easy way to do that.
> 
> > I'll have to look and see if there is an access function for this.
> 
> Great, thanks.

I would not mind re-exporting sysrq_on() again.

-- 
Dmitry

Re: linux-next: build failure after merge of the kgdb tree

[Jason Wessel]

On 05/20/2010 08:23 PM, Dmitry Torokhov wrote:
> On Fri, May 21, 2010 at 11:09:38AM +1000, Stephen Rothwell wrote:
>> Hi Jason,
>>
>> On Thu, 20 May 2010 19:49:51 -0500 Jason Wessel <jason.wessel@windriver.com> wrote:
>>> Before brute force toggling it, it seems we should check the value and
>>> restore it after the execution of handle_sysrq().
>> Indeed, at the time I couldn't find an easy way to do that.
>>
>>> I'll have to look and see if there is an access function for this.
>> Great, thanks.
> 
> I would not mind re-exporting sysrq_on() again.
> 

We could but I don't know that you need to.

Would you be willing to sign off on a change like the one below
Dmitry?  If so then I'll push it into kgdb-next.

It is as simple as making the return from sysrq_toggle_support a bit
more meaningful.

Thanks,
Jason.


--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -717,7 +717,7 @@ int sysrq_toggle_support(int enable_mask
 			sysrq_unregister_handler();
 	}
 
-	return 0;
+	return was_enabled;
 }
 
 static int __sysrq_swap_key_ops(int key, struct sysrq_key_op *insert_op_p,
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1926,17 +1926,19 @@ static int kdb_rm(int argc, const char *
  */
 static int kdb_sr(int argc, const char **argv)
 {
+	int toggle_save;
+
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	if (!__sysrq_enabled) {
-		kdb_printf("Auto activating sysrq\n");
-		__sysrq_enabled = 1;
-	}
+
+	toggle_save = sysrq_toggle_support(1);
 
 	kdb_trap_printk++;
 	handle_sysrq(*argv[1], NULL);
 	kdb_trap_printk--;
 
+	sysrq_toggle_support(toggle_save);
+
 	return 0;
 }
 #endif	/* CONFIG_MAGIC_SYSRQ */

Re: linux-next: build failure after merge of the kgdb tree

[Dmitry Torokhov]

On Thu, May 20, 2010 at 08:28:54PM -0500, Jason Wessel wrote:
> On 05/20/2010 08:23 PM, Dmitry Torokhov wrote:
> > On Fri, May 21, 2010 at 11:09:38AM +1000, Stephen Rothwell wrote:
> >> Hi Jason,
> >>
> >> On Thu, 20 May 2010 19:49:51 -0500 Jason Wessel <jason.wessel@windriver.com> wrote:
> >>> Before brute force toggling it, it seems we should check the value and
> >>> restore it after the execution of handle_sysrq().
> >> Indeed, at the time I couldn't find an easy way to do that.
> >>
> >>> I'll have to look and see if there is an access function for this.
> >> Great, thanks.
> > 
> > I would not mind re-exporting sysrq_on() again.
> > 
> 
> We could but I don't know that you need to.
> 
> Would you be willing to sign off on a change like the one below
> Dmitry?  If so then I'll push it into kgdb-next.
> 
> It is as simple as making the return from sysrq_toggle_support a bit
> more meaningful.
>

I do not think it is a very good idea... What if some other process
enales SysRq in the mean time. Do we really need to force SysRq on
or off? Maybe we should export __handle_sysrq() instead?

Also, I think I need to add locking in sysrq_toggle_support(), which
will make it unsuitable for using in kdb handler, won't it?

-- 
Dmitry

Re: linux-next: build failure after merge of the kgdb tree

[Jason Wessel]

On 05/24/2010 01:17 PM, Dmitry Torokhov wrote:

> Do we really need to force SysRq on or off? Maybe we should export
> __handle_sysrq() instead?


It seems cleanest to export __handle_sysrq() if that is ok?

> Also, I think I need to add locking in sysrq_toggle_support(), which
> will make it unsuitable for using in kdb handler, won't it?
> 

Any kind of locking would provide another opportunity to deadlock the
debugger.  It is probably worth noting that any of the sysrq
operations have the possibility to dead lock the system depending on
what type of exception was taken.  The idea is that you can use kdb in
one of these fatal exceptions and additionally run sysrq when you
might not have otherwise been able to before because the keyboard
handler was dead after a call to panic() for example.  At the point it
in time that it hangs there, the system was dead anyway. :-)


--------------

From: Jason Wessel <jason.wessel@windriver.com>
Subject: [PATCH] sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function

The kdb code should not toggle the sysrq state in case an end user
wants to try and resume the normal kernel execution.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 drivers/char/sysrq.c        |    2 +-
 include/linux/sysrq.h       |    1 +
 kernel/debug/kdb/kdb_main.c |    3 +--
 3 files changed, 3 insertions(+), 3 deletions(-)

--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -493,7 +493,7 @@ static void __sysrq_put_key_op(int key, 
                 sysrq_key_table[i] = op_p;
 }
 
-static void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
 {
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -45,6 +45,7 @@ struct sysrq_key_op {
  */
 
 void handle_sysrq(int key, struct tty_struct *tty);
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
 struct sysrq_key_op *__sysrq_get_key_op(int key);
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1820,9 +1820,8 @@ static int kdb_sr(int argc, const char *
 {
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	sysrq_toggle_support(1);
 	kdb_trap_printk++;
-	handle_sysrq(*argv[1], NULL);
+	__handle_sysrq(*argv[1], NULL, 0);
 	kdb_trap_printk--;
 
 	return 0;

Re: linux-next: build failure after merge of the kgdb tree

[Dmitry Torokhov]

On Monday 24 May 2010 01:04:17 pm Jason Wessel wrote:
> On 05/24/2010 01:17 PM, Dmitry Torokhov wrote:
> > Do we really need to force SysRq on or off? Maybe we should export
> > __handle_sysrq() instead?
> 
> It seems cleanest to export __handle_sysrq() if that is ok?

Yes, I think this is the best option. Please feel free to merge through
your tree.

Thanks.

-- 
Dmitry

Re: linux-next: build failure after merge of the kgdb tree

[Jason Wessel]

On 05/24/2010 03:34 PM, Dmitry Torokhov wrote:
> On Monday 24 May 2010 01:04:17 pm Jason Wessel wrote:
>   
>> On 05/24/2010 01:17 PM, Dmitry Torokhov wrote:
>>     
>>> Do we really need to force SysRq on or off? Maybe we should export
>>> __handle_sysrq() instead?
>>>       
>> It seems cleanest to export __handle_sysrq() if that is ok?
>>     
>
> Yes, I think this is the best option. Please feel free to merge through
> your tree.
>   

Will do, I'll add your ack and queue it for the minor -rc1 fixes I am
accumulating for kgdb/kdb.

Thanks,
Jason.

linux-next: build failure after merge of the kgdb tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]

Hi all,

After merging the kgdb tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/tty/serial/kgdboc.c:418:13: error: expected declaration specifiers or '...' before string constant
  418 | early_param("ekgdboc", kgdboc_early_init);
      |             ^~~~~~~~~
drivers/tty/serial/kgdboc.c:418:24: error: expected declaration specifiers or '...' before 'kgdboc_early_init'
  418 | early_param("ekgdboc", kgdboc_early_init);
      |                        ^~~~~~~~~~~~~~~~~
drivers/tty/serial/kgdboc.c:546:13: error: expected declaration specifiers or '...' before string constant
  546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
      |             ^~~~~~~~~~~~~~~~~
drivers/tty/serial/kgdboc.c:546:32: error: expected declaration specifiers or '...' before 'kgdboc_earlycon_init'
  546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
      |                                ^~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/kgdboc.c:505:19: warning: 'kgdboc_earlycon_init' defined but not used [-Wunused-function]
  505 | static int __init kgdboc_earlycon_init(char *opt)
      |                   ^~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/kgdboc.c:411:19: warning: 'kgdboc_early_init' defined but not used [-Wunused-function]
  411 | static int __init kgdboc_early_init(char *opt)
      |                   ^~~~~~~~~~~~~~~~~

Caused by commit

  220995622da5 ("kgdboc: Add kgdboc_earlycon to support early kgdb using boot consoles")

I have used the kgdb tree from next-20200518 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the kgdb tree

[Doug Anderson]

Hi,

On Tue, May 19, 2020 at 7:05 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the kgdb tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> drivers/tty/serial/kgdboc.c:418:13: error: expected declaration specifiers or '...' before string constant
>   418 | early_param("ekgdboc", kgdboc_early_init);
>       |             ^~~~~~~~~
> drivers/tty/serial/kgdboc.c:418:24: error: expected declaration specifiers or '...' before 'kgdboc_early_init'
>   418 | early_param("ekgdboc", kgdboc_early_init);
>       |                        ^~~~~~~~~~~~~~~~~
> drivers/tty/serial/kgdboc.c:546:13: error: expected declaration specifiers or '...' before string constant
>   546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
>       |             ^~~~~~~~~~~~~~~~~
> drivers/tty/serial/kgdboc.c:546:32: error: expected declaration specifiers or '...' before 'kgdboc_earlycon_init'
>   546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
>       |                                ^~~~~~~~~~~~~~~~~~~~
> drivers/tty/serial/kgdboc.c:505:19: warning: 'kgdboc_earlycon_init' defined but not used [-Wunused-function]
>   505 | static int __init kgdboc_earlycon_init(char *opt)
>       |                   ^~~~~~~~~~~~~~~~~~~~
> drivers/tty/serial/kgdboc.c:411:19: warning: 'kgdboc_early_init' defined but not used [-Wunused-function]
>   411 | static int __init kgdboc_early_init(char *opt)
>       |                   ^~~~~~~~~~~~~~~~~
>
> Caused by commit
>
>   220995622da5 ("kgdboc: Add kgdboc_earlycon to support early kgdb using boot consoles")
>
> I have used the kgdb tree from next-20200518 for today.

Ugh.  Actually, I think the commit to blame is:

eae3e19ca930 ("kgdboc: Remove useless #ifdef
CONFIG_KGDB_SERIAL_CONSOLE in kgdboc")

The next commit just made it worse.  Apparently the #ifdef wasn't so
useless after all.  It was just subtly keeping the code from compiling
when kgdboc was used as a module.  That's because when it's a module
we instead get this defined:

#define CONFIG_KGDB_SERIAL_CONSOLE_MODULE 1

Apparently I didn't re-test as a module after I made this change in
one of the later spins.  :(

I think I can whip up a quick patch that uses "IS_BUILTIN(option)".
Basically this should go back to how the code was in one of the
earlier patchsets where I tested this.

-Doug

Re: linux-next: build failure after merge of the kgdb tree

[Doug Anderson]

Hi,

On Tue, May 19, 2020 at 7:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, May 19, 2020 at 7:05 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> >
> > After merging the kgdb tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > drivers/tty/serial/kgdboc.c:418:13: error: expected declaration specifiers or '...' before string constant
> >   418 | early_param("ekgdboc", kgdboc_early_init);
> >       |             ^~~~~~~~~
> > drivers/tty/serial/kgdboc.c:418:24: error: expected declaration specifiers or '...' before 'kgdboc_early_init'
> >   418 | early_param("ekgdboc", kgdboc_early_init);
> >       |                        ^~~~~~~~~~~~~~~~~
> > drivers/tty/serial/kgdboc.c:546:13: error: expected declaration specifiers or '...' before string constant
> >   546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
> >       |             ^~~~~~~~~~~~~~~~~
> > drivers/tty/serial/kgdboc.c:546:32: error: expected declaration specifiers or '...' before 'kgdboc_earlycon_init'
> >   546 | early_param("kgdboc_earlycon", kgdboc_earlycon_init);
> >       |                                ^~~~~~~~~~~~~~~~~~~~
> > drivers/tty/serial/kgdboc.c:505:19: warning: 'kgdboc_earlycon_init' defined but not used [-Wunused-function]
> >   505 | static int __init kgdboc_earlycon_init(char *opt)
> >       |                   ^~~~~~~~~~~~~~~~~~~~
> > drivers/tty/serial/kgdboc.c:411:19: warning: 'kgdboc_early_init' defined but not used [-Wunused-function]
> >   411 | static int __init kgdboc_early_init(char *opt)
> >       |                   ^~~~~~~~~~~~~~~~~
> >
> > Caused by commit
> >
> >   220995622da5 ("kgdboc: Add kgdboc_earlycon to support early kgdb using boot consoles")
> >
> > I have used the kgdb tree from next-20200518 for today.
>
> Ugh.  Actually, I think the commit to blame is:
>
> eae3e19ca930 ("kgdboc: Remove useless #ifdef
> CONFIG_KGDB_SERIAL_CONSOLE in kgdboc")
>
> The next commit just made it worse.  Apparently the #ifdef wasn't so
> useless after all.  It was just subtly keeping the code from compiling
> when kgdboc was used as a module.  That's because when it's a module
> we instead get this defined:
>
> #define CONFIG_KGDB_SERIAL_CONSOLE_MODULE 1
>
> Apparently I didn't re-test as a module after I made this change in
> one of the later spins.  :(
>
> I think I can whip up a quick patch that uses "IS_BUILTIN(option)".
> Basically this should go back to how the code was in one of the
> earlier patchsets where I tested this.

Fix posted at:

https://lore.kernel.org/r/20200519084345.1.I91670accc8a5ddabab227eb63bb4ad3e2e9d2b58@changeid

I have confirmed:

* x86_64 allmodconfig atop the kgdb tree compiles.

* Building and using kgdboc as a module works with this patch on my
local 5.4-based tree.  Loaded the module both with module parameters
and via echoing to sysfs.  Unloaded the module and confirmed it works.

* Building and using kgdboc builtin allows me to do kgdb_earlycon on
my local 5.4-based tree.


-Doug