Signed-off-by: liguang <[email protected]>
---
kernel/sys.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index b95d3c7..07c6177 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -151,9 +151,9 @@ static bool set_one_prio_perm(struct task_struct *p)
* set the priority of a task
* - the caller must hold the RCU read lock
*/
-static int set_one_prio(struct task_struct *p, int niceval, int error)
+static int set_one_prio(struct task_struct *p, int niceval)
{
- int no_nice;
+ int no_nice, error = 0;
if (!set_one_prio_perm(p)) {
error = -EPERM;
@@ -168,8 +168,6 @@ static int set_one_prio(struct task_struct *p, int niceval, int error)
error = no_nice;
goto out;
}
- if (error == -ESRCH)
- error = 0;
set_user_nice(p, niceval);
out:
return error;
@@ -203,7 +201,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
else
p = current;
if (p)
- error = set_one_prio(p, niceval, error);
+ error = set_one_prio(p, niceval);
break;
case PRIO_PGRP:
if (who)
@@ -211,7 +209,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
else
pgrp = task_pgrp(current);
do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
- error = set_one_prio(p, niceval, error);
+ error = set_one_prio(p, niceval);
} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
break;
case PRIO_USER:
@@ -225,7 +223,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
do_each_thread(g, p) {
if (uid_eq(task_uid(p), uid))
- error = set_one_prio(p, niceval, error);
+ error = set_one_prio(p, niceval);
} while_each_thread(g, p);
if (!uid_eq(uid, cred->uid))
free_uid(user); /* For find_user() */
--
1.7.2.5
Signed-off-by: liguang <[email protected]>
---
include/linux/reboot.h | 2 +-
kernel/sys.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 23b3630..a8c5e4c 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -35,7 +35,7 @@ extern void kernel_restart(char *cmd);
extern void kernel_halt(void);
extern void kernel_power_off(void);
-extern int C_A_D; /* for sysctl */
+extern bool C_A_D; /* for sysctl */
void ctrl_alt_del(void);
#define POWEROFF_CMD_PATH_LEN 256
diff --git a/kernel/sys.c b/kernel/sys.c
index 7f56ed4..6dabfc1 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -119,7 +119,7 @@ EXPORT_SYMBOL(fs_overflowgid);
* this indicates whether you can reboot with ctrl-alt-del: the default is yes
*/
-int C_A_D = 1;
+bool C_A_D = true;
struct pid *cad_pid;
EXPORT_SYMBOL(cad_pid);
@@ -475,11 +475,11 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
break;
case LINUX_REBOOT_CMD_CAD_ON:
- C_A_D = 1;
+ C_A_D = true;
break;
case LINUX_REBOOT_CMD_CAD_OFF:
- C_A_D = 0;
+ C_A_D = false;
break;
case LINUX_REBOOT_CMD_HALT:
--
1.7.2.5
if LINUX_REBOOT_CMD_HALT for reboot failed,
message "cannot halt" will stay in same line
with next message, so append a '\n' for it.
Signed-off-by: liguang <[email protected]>
---
kernel/sys.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 6dabfc1..c04c611 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -486,7 +486,7 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
system_state = SYSTEM_HALT;
kernel_halt();
do_exit(0);
- panic("cannot halt");
+ panic("cannot halt.\n");
case LINUX_REBOOT_CMD_POWER_OFF:
system_state = SYSTEM_POWER_OFF;
--
1.7.2.5
kernel_shutdown_prepare's parameter can be removed
by checking global 'system_state', so, maybe we
can save a register to be used :-).
Signed-off-by: liguang <[email protected]>
---
kernel/sys.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 07c6177..7f56ed4 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -382,11 +382,10 @@ void kernel_restart(char *cmd)
}
EXPORT_SYMBOL_GPL(kernel_restart);
-static void kernel_shutdown_prepare(enum system_states state)
+static void kernel_shutdown_prepare(void)
{
blocking_notifier_call_chain(&reboot_notifier_list,
- (state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
- system_state = state;
+ (system_state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
usermodehelper_disable();
device_shutdown();
}
@@ -397,7 +396,7 @@ static void kernel_shutdown_prepare(enum system_states state)
*/
void kernel_halt(void)
{
- kernel_shutdown_prepare(SYSTEM_HALT);
+ kernel_shutdown_prepare();
disable_nonboot_cpus();
syscore_shutdown();
printk(KERN_EMERG "System halted.\n");
@@ -414,7 +413,7 @@ EXPORT_SYMBOL_GPL(kernel_halt);
*/
void kernel_power_off(void)
{
- kernel_shutdown_prepare(SYSTEM_POWER_OFF);
+ kernel_shutdown_prepare();
if (pm_power_off_prepare)
pm_power_off_prepare();
disable_nonboot_cpus();
@@ -484,11 +483,13 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
break;
case LINUX_REBOOT_CMD_HALT:
+ system_state = SYSTEM_HALT;
kernel_halt();
do_exit(0);
panic("cannot halt");
case LINUX_REBOOT_CMD_POWER_OFF:
+ system_state = SYSTEM_POWER_OFF;
kernel_power_off();
do_exit(0);
break;
--
1.7.2.5
On Thu, 2013-05-30 at 15:58 +0800, liguang wrote:
[]
> diff --git a/kernel/sys.c b/kernel/sys.c
[]
> -static int set_one_prio(struct task_struct *p, int niceval, int error)
> +static int set_one_prio(struct task_struct *p, int niceval)
Umm, error is forwarded through do_each loops.
Are you sure you can do this without
changing any return code behaviors?
[]
> @@ -211,7 +209,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
> else
> pgrp = task_pgrp(current);
> do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> - error = set_one_prio(p, niceval, error);
> + error = set_one_prio(p, niceval);
> } while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> break;
> case PRIO_USER:
> @@ -225,7 +223,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
>
> do_each_thread(g, p) {
> if (uid_eq(task_uid(p), uid))
> - error = set_one_prio(p, niceval, error);
> + error = set_one_prio(p, niceval);
> } while_each_thread(g, p);
> if (!uid_eq(uid, cred->uid))
> free_uid(user); /* For find_user() */
On Thu, 30 May 2013 15:58:03 +0800 liguang <[email protected]> wrote:
> Signed-off-by: liguang <[email protected]>
> ---
> kernel/sys.c | 12 +++++-------
> 1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index b95d3c7..07c6177 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -151,9 +151,9 @@ static bool set_one_prio_perm(struct task_struct *p)
> * set the priority of a task
> * - the caller must hold the RCU read lock
> */
> -static int set_one_prio(struct task_struct *p, int niceval, int error)
> +static int set_one_prio(struct task_struct *p, int niceval)
> {
> - int no_nice;
> + int no_nice, error = 0;
>
> if (!set_one_prio_perm(p)) {
> error = -EPERM;
> @@ -168,8 +168,6 @@ static int set_one_prio(struct task_struct *p, int niceval, int error)
> error = no_nice;
> goto out;
> }
> - if (error == -ESRCH)
> - error = 0;
> set_user_nice(p, niceval);
> out:
> return error;
> @@ -203,7 +201,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
> else
> p = current;
> if (p)
> - error = set_one_prio(p, niceval, error);
> + error = set_one_prio(p, niceval);
> break;
> case PRIO_PGRP:
> if (who)
> @@ -211,7 +209,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
> else
> pgrp = task_pgrp(current);
> do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> - error = set_one_prio(p, niceval, error);
> + error = set_one_prio(p, niceval);
> } while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> break;
> case PRIO_USER:
> @@ -225,7 +223,7 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
>
> do_each_thread(g, p) {
> if (uid_eq(task_uid(p), uid))
> - error = set_one_prio(p, niceval, error);
> + error = set_one_prio(p, niceval);
> } while_each_thread(g, p);
> if (!uid_eq(uid, cred->uid))
> free_uid(user); /* For find_user() */
Yes, that apepars to be an evuivalent change, but only because
security_task_setnice() cannot return -ESRCH. The existing code is
rather awkward.
A couple of changes:
From: Andrew Morton <[email protected]>
Subject: kernel-sysc-remove-unnecessary-parameter-of-set_one_prio-fix
clean up definitions, remove unneeded assignment
Cc: liguang <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
kernel/sys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -puN kernel/sys.c~kernel-sysc-remove-unnecessary-parameter-of-set_one_prio-fix kernel/sys.c
--- a/kernel/sys.c~kernel-sysc-remove-unnecessary-parameter-of-set_one_prio-fix
+++ a/kernel/sys.c
@@ -153,7 +153,8 @@ static bool set_one_prio_perm(struct tas
*/
static int set_one_prio(struct task_struct *p, int niceval)
{
- int no_nice, error = 0;
+ int no_nice;
+ int error = 0;
if (!set_one_prio_perm(p)) {
error = -EPERM;
@@ -186,7 +187,6 @@ SYSCALL_DEFINE3(setpriority, int, which,
goto out;
/* normalize: avoid signed division (rounding problems) */
- error = -ESRCH;
if (niceval < -20)
niceval = -20;
if (niceval > 19)
_
On Thu, 30 May 2013 15:58:04 +0800 liguang <[email protected]> wrote:
> kernel_shutdown_prepare's parameter can be removed
> by checking global 'system_state', so, maybe we
> can save a register to be used :-).
>
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -382,11 +382,10 @@ void kernel_restart(char *cmd)
> }
> EXPORT_SYMBOL_GPL(kernel_restart);
>
> -static void kernel_shutdown_prepare(enum system_states state)
> +static void kernel_shutdown_prepare(void)
> {
> blocking_notifier_call_chain(&reboot_notifier_list,
> - (state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
> - system_state = state;
> + (system_state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
> usermodehelper_disable();
> device_shutdown();
> }
> @@ -397,7 +396,7 @@ static void kernel_shutdown_prepare(enum system_states state)
> */
> void kernel_halt(void)
> {
> - kernel_shutdown_prepare(SYSTEM_HALT);
> + kernel_shutdown_prepare();
> disable_nonboot_cpus();
> syscore_shutdown();
> printk(KERN_EMERG "System halted.\n");
> @@ -414,7 +413,7 @@ EXPORT_SYMBOL_GPL(kernel_halt);
> */
> void kernel_power_off(void)
> {
> - kernel_shutdown_prepare(SYSTEM_POWER_OFF);
> + kernel_shutdown_prepare();
> if (pm_power_off_prepare)
> pm_power_off_prepare();
> disable_nonboot_cpus();
> @@ -484,11 +483,13 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
> break;
>
> case LINUX_REBOOT_CMD_HALT:
> + system_state = SYSTEM_HALT;
> kernel_halt();
> do_exit(0);
> panic("cannot halt");
>
> case LINUX_REBOOT_CMD_POWER_OFF:
> + system_state = SYSTEM_POWER_OFF;
> kernel_power_off();
> do_exit(0);
> break;
No, this changes behavior for all other callers of kernel_halt() and
kernel_power_off(): system_state will no longer be updated for those
callers.
And passing an argument via a global variable is not nice :(
On Thu, 30 May 2013 15:58:05 +0800 liguang <[email protected]> wrote:
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -35,7 +35,7 @@ extern void kernel_restart(char *cmd);
> extern void kernel_halt(void);
> extern void kernel_power_off(void);
>
> -extern int C_A_D; /* for sysctl */
> +extern bool C_A_D; /* for sysctl */
> void ctrl_alt_del(void);
This means that the pointer in kernel/sysctl.c:kern_table.data now
points at a bool but is declared to have size sizeof(int).
That happens to work with current gcc verions, but there's no rule
which states that sizeof(bool) must equal sizeof(int).
And I'm not sure that changing kern_table to use sizeof(bool) is really
worth all the bother.
On Thu, 30 May 2013 01:08:25 -0700 Joe Perches <[email protected]> wrote:
> On Thu, 2013-05-30 at 15:58 +0800, liguang wrote:
> []
> > diff --git a/kernel/sys.c b/kernel/sys.c
> []
> > -static int set_one_prio(struct task_struct *p, int niceval, int error)
> > +static int set_one_prio(struct task_struct *p, int niceval)
>
> Umm, error is forwarded through do_each loops.
>
> Are you sure you can do this without
> changing any return code behaviors?
yes, you're right. The first time around, -ESRCH gets rewritten to 0
and the zero propagates, as long as nobody hits an error which isn't
-ESRCH. I'm not sure this was the most straightforward possible
implementation :(
在 2013-05-31五的 16:02 -0700，Andrew Morton写道：
> On Thu, 30 May 2013 15:58:05 +0800 liguang <[email protected]> wrote:
>
> > --- a/include/linux/reboot.h
> > +++ b/include/linux/reboot.h
> > @@ -35,7 +35,7 @@ extern void kernel_restart(char *cmd);
> > extern void kernel_halt(void);
> > extern void kernel_power_off(void);
> >
> > -extern int C_A_D; /* for sysctl */
> > +extern bool C_A_D; /* for sysctl */
> > void ctrl_alt_del(void);
>
> This means that the pointer in kernel/sysctl.c:kern_table.data now
> points at a bool but is declared to have size sizeof(int).
>
> That happens to work with current gcc verions, but there's no rule
> which states that sizeof(bool) must equal sizeof(int).
>
> And I'm not sure that changing kern_table to use sizeof(bool) is really
> worth all the bother.
OK, got it,
Thanks!
On Fri, 2013-05-31 at 16:02 -0700, Andrew Morton wrote:
> there's no rule
> which states that sizeof(bool) must equal sizeof(int).
For gcc, sizeof(_Bool) isn't sizeof(int) so it would
work for one endian but not the other.
gcc has sizeof(_Bool) == sizeof(unsigned char)