[PATCH] sysrq, intel_fb: fix sysrq g collision

[PATCH] sysrq, intel_fb: fix sysrq g collision

[Jason Wessel]

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


This is patch is both a clean up and a regression fix because two things
in the mainline kernel can register for sysrq-g, which is certainly a
problem when you want to use both.

I talked to Randy and Jesse briefly about changing the intel fb video
driver to use 'v' instead of 'g' and they agreed it would be ok to see a
patch.  Hopefully each will ack this patch, or discuss it further.   If
they ack the patch, I can send it on to Linus for consideration.

The long term intent here is to get mode switching with a generic kernel
debug front end, with the short term intent to fix the sysrq collision.

Thanks,
Jason.

[-- Attachment #2: 0001-Cleanup-sysrq.patch --]
[-- Type: text/x-diff, Size: 2972 bytes --]

From: Jason Wessel <jason.wessel@windriver.com>
Date: Wed, 13 May 2009 21:56:59 -0500
Subject: [PATCH] sysrq, intel_fb: fix sysrq g collision

Commit 79e539453b34e35f39299a899d263b0a1f1670bd introduced a
regression where you cannot use sysrq 'g' to enter kgdb.  The solution
is to move the intel fb sysrq over to V for video instead of G for
graphics.  The SMP VOYAGER code to register for the sysrq-v is not
anywhere to be found in the mainline kernel, so the comments in the
code were cleaned up as well.

This patch also cleans up the sysrq definitions for kgdb to make it
generic for the kernel debugger, such that the sysrq 'g' can be used
in the future to enter a gdbstub or another kernel debugger.

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

---
 drivers/char/sysrq.c            |    4 ++--
 drivers/gpu/drm/i915/intel_fb.c |    4 ++--
 kernel/kgdb.c                   |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index b0a6a3e..d6a807f 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -406,7 +406,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	&sysrq_showlocks_op,		/* d */
 	&sysrq_term_op,			/* e */
 	&sysrq_moom_op,			/* f */
-	/* g: May be registered by ppc for kgdb */
+	/* g: May be registered for the kernel debugger */
 	NULL,				/* g */
 	NULL,				/* h - reserved for help */
 	&sysrq_kill_op,			/* i */
@@ -431,7 +431,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	&sysrq_sync_op,			/* s */
 	&sysrq_showstate_op,		/* t */
 	&sysrq_mountro_op,		/* u */
-	/* v: May be registered at init time by SMP VOYAGER */
+	/* v: May be registered for frame buffer video console restore */
 	NULL,				/* v */
 	&sysrq_showstate_blocked_op,	/* w */
 	/* x: May be registered on ppc/powerpc for xmon */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 3e094be..e4652dc 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -864,7 +864,7 @@ static void intelfb_sysrq(int dummy1, struct tty_struct *dummy3)
 
 static struct sysrq_key_op sysrq_intelfb_restore_op = {
         .handler = intelfb_sysrq,
-        .help_msg = "force-fb(G)",
+        .help_msg = "force-fb(V)",
         .action_msg = "Restore framebuffer console",
 };
 
@@ -898,7 +898,7 @@ int intelfb_probe(struct drm_device *dev)
 		ret = intelfb_single_fb_probe(dev);
 	}
 
-	register_sysrq_key('g', &sysrq_intelfb_restore_op);
+	register_sysrq_key('v', &sysrq_intelfb_restore_op);
 
 	return ret;
 }
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index e4dcfb2..9147a31 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -1583,8 +1583,8 @@ static void sysrq_handle_gdb(int key, struct tty_struct *tty)
 
 static struct sysrq_key_op sysrq_gdb_op = {
 	.handler	= sysrq_handle_gdb,
-	.help_msg	= "Gdb",
-	.action_msg	= "GDB",
+	.help_msg	= "debug(G)",
+	.action_msg	= "DEBUG",
 };
 #endif
 
-- 
1.6.3.9.g6345d

Re: [PATCH] sysrq, intel_fb: fix sysrq g collision

[Jesse Barnes]

On Thu, 14 May 2009 17:58:11 -0500
Jason Wessel <jason.wessel@windriver.com> wrote:

> 
> This is patch is both a clean up and a regression fix because two
> things in the mainline kernel can register for sysrq-g, which is
> certainly a problem when you want to use both.
> 
> I talked to Randy and Jesse briefly about changing the intel fb video
> driver to use 'v' instead of 'g' and they agreed it would be ok to
> see a patch.  Hopefully each will ack this patch, or discuss it
> further.   If they ack the patch, I can send it on to Linus for
> consideration.
> 
> The long term intent here is to get mode switching with a generic
> kernel debug front end, with the short term intent to fix the sysrq
> collision.

Looks good, thanks Jason.

Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] sysrq, intel_fb: fix sysrq g collision

[Randy Dunlap]

Jason Wessel wrote:
> This is patch is both a clean up and a regression fix because two things
> in the mainline kernel can register for sysrq-g, which is certainly a
> problem when you want to use both.
> 
> I talked to Randy and Jesse briefly about changing the intel fb video
> driver to use 'v' instead of 'g' and they agreed it would be ok to see a
> patch.  Hopefully each will ack this patch, or discuss it further.   If
> they ack the patch, I can send it on to Linus for consideration.
> 
> The long term intent here is to get mode switching with a generic kernel
> debug front end, with the short term intent to fix the sysrq collision.

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Thanks.

-- 
~Randy
LPC 2009, Sept. 23-25, Portland, Oregon
http://linuxplumbersconf.org/2009/

Re: [PATCH] sysrq, intel_fb: fix sysrq g collision

[Andrew Morton]

On Thu, 14 May 2009 17:58:11 -0500
Jason Wessel <jason.wessel@windriver.com> wrote:

> Commit 79e539453b34e35f39299a899d263b0a1f1670bd introduced a
> regression where you cannot use sysrq 'g' to enter kgdb.  The solution
> is to move the intel fb sysrq over to V for video instead of G for
> graphics.  The SMP VOYAGER code to register for the sysrq-v is not
> anywhere to be found in the mainline kernel, so the comments in the
> code were cleaned up as well.
> 
> This patch also cleans up the sysrq definitions for kgdb to make it
> generic for the kernel debugger, such that the sysrq 'g' can be used
> in the future to enter a gdbstub or another kernel debugger.
> 

I was looking at that a week or two ago.

> 
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/sysrq.c
> @@ -406,7 +406,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
>  	&sysrq_showlocks_op,		/* d */
>  	&sysrq_term_op,			/* e */
>  	&sysrq_moom_op,			/* f */
> -	/* g: May be registered by ppc for kgdb */
> +	/* g: May be registered for the kernel debugger */

And I made the mistake of believing the comment :(