[PATCH 0/1] sysrq is half-enabled by default V2

[PATCH 0/1] sysrq is half-enabled by default V2

[Andy Whitcroft]

It seems that recent cleanups to sysrq handling have lead to us having
two copies of the sysrq enable status.  At boot these are currently
inconsistently specified leading to sysrq actually being enabled but
reported as not enabled in sysfs.  Following this email is a patch to
link both of these copies to a common header define to prevent them becoming
out of sync again.

-apw

Andy Whitcroft (1):
  Input: sysrq -- ensure sysrq_enabled and __sysrq_enabled are
    consistent

 drivers/tty/sysrq.c   |    2 +-
 include/linux/sysrq.h |    3 +++
 kernel/sysctl.c       |    3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

[PATCH 1/1] Input: sysrq -- ensure sysrq_enabled and __sysrq_enabled are consistent

[Andy Whitcroft]

Currently sysrq_enabled and __sysrq_enabled are initialised separatly
and inconsitantly, leading to sysrq being actually enabled by reported
as not enabled in sysfs.  The first change to the sysfs configurable
synchronises these two:

    static int __read_mostly sysrq_enabled = 1;
    static int __sysrq_enabled;

Add a common define to carry the default for these preventing them becoming
out of sync again.  Default this to 1 to mirror previous behaviour.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 drivers/tty/sysrq.c   |    2 +-
 include/linux/sysrq.h |    3 +++
 kernel/sysctl.c       |    3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c556ed9..8e0dd25 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -46,7 +46,7 @@
 #include <asm/irq_regs.h>
 
 /* Whether we react on sysrq keys or just ignore them */
-static int __read_mostly sysrq_enabled = 1;
+static int __read_mostly sysrq_enabled = SYSRQ_DEFAULT_ENABLE;
 static bool __read_mostly sysrq_always_enabled;
 
 static bool sysrq_on(void)
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 387fa7d..7faf933 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -17,6 +17,9 @@
 #include <linux/errno.h>
 #include <linux/types.h>
 
+/* Enable/disable SYSRQ support by default (0==no, 1==yes). */
+#define SYSRQ_DEFAULT_ENABLE	1
+
 /* Possible values of bitmask for enabling sysrq functions */
 /* 0x0001 is reserved for enable everything */
 #define SYSRQ_ENABLE_LOG	0x0002
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index bc86bb3..0f1bd83 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -170,7 +170,8 @@ static int proc_taint(struct ctl_table *table, int write,
 #endif
 
 #ifdef CONFIG_MAGIC_SYSRQ
-static int __sysrq_enabled; /* Note: sysrq code ises it's own private copy */
+/* Note: sysrq code uses it's own private copy */
+static int __sysrq_enabled = SYSRQ_DEFAULT_ENABLE;
 
 static int sysrq_sysctl_handler(ctl_table *table, int write,
 				void __user *buffer, size_t *lenp,
-- 
1.7.1

Re: [PATCH 1/1] Input: sysrq -- ensure sysrq_enabled and __sysrq_enabled are consistent

[Randy Dunlap]

On Mon, 24 Jan 2011 14:06:12 +0000 Andy Whitcroft wrote:

> Currently sysrq_enabled and __sysrq_enabled are initialised separatly
> and inconsitantly, leading to sysrq being actually enabled by reported
> as not enabled in sysfs.  The first change to the sysfs configurable
> synchronises these two:
> 
>     static int __read_mostly sysrq_enabled = 1;
>     static int __sysrq_enabled;
> 
> Add a common define to carry the default for these preventing them becoming
> out of sync again.  Default this to 1 to mirror previous behaviour.
> 
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
>  drivers/tty/sysrq.c   |    2 +-
>  include/linux/sysrq.h |    3 +++
>  kernel/sysctl.c       |    3 ++-
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index c556ed9..8e0dd25 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -46,7 +46,7 @@
>  #include <asm/irq_regs.h>
>  
>  /* Whether we react on sysrq keys or just ignore them */
> -static int __read_mostly sysrq_enabled = 1;
> +static int __read_mostly sysrq_enabled = SYSRQ_DEFAULT_ENABLE;
>  static bool __read_mostly sysrq_always_enabled;
>  
>  static bool sysrq_on(void)
> diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
> index 387fa7d..7faf933 100644
> --- a/include/linux/sysrq.h
> +++ b/include/linux/sysrq.h
> @@ -17,6 +17,9 @@
>  #include <linux/errno.h>
>  #include <linux/types.h>
>  
> +/* Enable/disable SYSRQ support by default (0==no, 1==yes). */
> +#define SYSRQ_DEFAULT_ENABLE	1
> +
>  /* Possible values of bitmask for enabling sysrq functions */
>  /* 0x0001 is reserved for enable everything */
>  #define SYSRQ_ENABLE_LOG	0x0002
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index bc86bb3..0f1bd83 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -170,7 +170,8 @@ static int proc_taint(struct ctl_table *table, int write,
>  #endif
>  
>  #ifdef CONFIG_MAGIC_SYSRQ
> -static int __sysrq_enabled; /* Note: sysrq code ises it's own private copy */
> +/* Note: sysrq code uses it's own private copy */

                            its

Meta-comment:  There's no good reason to have patch 0/1.  Just put all of the
patch description in patch 1/1.


> +static int __sysrq_enabled = SYSRQ_DEFAULT_ENABLE;
>  
>  static int sysrq_sysctl_handler(ctl_table *table, int write,
>  				void __user *buffer, size_t *lenp,
> -- 


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 1/1] Input: sysrq -- ensure sysrq_enabled and __sysrq_enabled are consistent

[Dmitry Torokhov]

On Mon, Jan 24, 2011 at 02:06:12PM +0000, Andy Whitcroft wrote:
> Currently sysrq_enabled and __sysrq_enabled are initialised separatly
> and inconsitantly, leading to sysrq being actually enabled by reported
> as not enabled in sysfs.  The first change to the sysfs configurable
> synchronises these two:
> 
>     static int __read_mostly sysrq_enabled = 1;
>     static int __sysrq_enabled;
> 
> Add a common define to carry the default for these preventing them becoming
> out of sync again.  Default this to 1 to mirror previous behaviour.
> 
> Signed-off-by: Andy Whitcroft <apw@canonical.com>

Applied and will also tag for stable. Thank you Andy.

-- 
Dmitry