From: Oleg Nesterov <oleg@redhat.com>
To: Roland McGrath <roland@redhat.com>
Cc: linux-kernel@vger.kernel.org
Subject: [FOR REVIEW, PATCH 2/2] introduce "struct wait_opts" to simplify do_wait() pathes
Date: Wed, 6 May 2009 07:33:24 +0200 [thread overview]
Message-ID: <20090506053324.GA31988@redhat.com> (raw)
Introduce "struct wait_opts" which holds the parameters for misc helpers
in do_wait() pathes.
This adds 17 lines to kernel/exit.c, but saves 256 bytes from .o and imho
makes the code much more readable.
(untested, not signed)
kernel/exit.c | 211 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 114 insertions(+), 97 deletions(-)
--- PTRACE/kernel/exit.c~2_WOPTS 2009-05-06 01:48:51.000000000 +0200
+++ PTRACE/kernel/exit.c 2009-05-06 07:00:45.000000000 +0200
@@ -1076,6 +1076,18 @@ SYSCALL_DEFINE1(exit_group, int, error_c
return 0;
}
+struct wait_opts {
+ enum pid_type wtype;
+ struct pid *wpid;
+ int wflags;
+
+ struct siginfo __user *winfo;
+ int __user *wstat;
+ struct rusage __user *wrusage;
+
+ int notask_error;
+};
+
static struct pid *task_pid_type(struct task_struct *task, enum pid_type type)
{
struct pid *pid = NULL;
@@ -1086,13 +1098,12 @@ static struct pid *task_pid_type(struct
return pid;
}
-static int eligible_child(enum pid_type type, struct pid *pid, int options,
- struct task_struct *p)
+static int eligible_child(struct wait_opts *wopts, struct task_struct *p)
{
int err;
- if (type < PIDTYPE_MAX) {
- if (task_pid_type(p, type) != pid)
+ if (wopts->wtype < PIDTYPE_MAX) {
+ if (task_pid_type(p, wopts->wtype) != wopts->wpid)
return 0;
}
@@ -1101,8 +1112,8 @@ static int eligible_child(enum pid_type
* set; otherwise, wait for non-clone children *only*. (Note:
* A "clone" child here is one that reports to its parent
* using a signal other than SIGCHLD.) */
- if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0))
- && !(options & __WALL))
+ if (((p->exit_signal != SIGCHLD) ^ !!(wopts->wflags & __WCLONE))
+ && !(wopts->wflags & __WALL))
return 0;
err = security_task_wait(p);
@@ -1112,14 +1123,15 @@ static int eligible_child(enum pid_type
return 1;
}
-static int wait_noreap_copyout(struct task_struct *p, pid_t pid, uid_t uid,
- int why, int status,
- struct siginfo __user *infop,
- struct rusage __user *rusagep)
+static int wait_noreap_copyout(struct wait_opts *wopts, struct task_struct *p,
+ pid_t pid, uid_t uid, int why, int status)
{
- int retval = rusagep ? getrusage(p, RUSAGE_BOTH, rusagep) : 0;
+ struct siginfo __user *infop;
+ int retval = wopts->wrusage
+ ? getrusage(p, RUSAGE_BOTH, wopts->wrusage) : 0;
put_task_struct(p);
+ infop = wopts->winfo;
if (!retval)
retval = put_user(SIGCHLD, &infop->si_signo);
if (!retval)
@@ -1143,19 +1155,18 @@ static int wait_noreap_copyout(struct ta
* the lock and this task is uninteresting. If we return nonzero, we have
* released the lock and the system call should return.
*/
-static int wait_task_zombie(struct task_struct *p, int options,
- struct siginfo __user *infop,
- int __user *stat_addr, struct rusage __user *ru)
+static int wait_task_zombie(struct wait_opts *wopts, struct task_struct *p)
{
unsigned long state;
int retval, status, traced;
pid_t pid = task_pid_vnr(p);
uid_t uid = __task_cred(p)->uid;
+ struct siginfo __user *infop;
- if (!likely(options & WEXITED))
+ if (!likely(wopts->wflags & WEXITED))
return 0;
- if (unlikely(options & WNOWAIT)) {
+ if (unlikely(wopts->wflags & WNOWAIT)) {
int exit_code = p->exit_code;
int why, status;
@@ -1168,8 +1179,7 @@ static int wait_task_zombie(struct task_
why = (exit_code & 0x80) ? CLD_DUMPED : CLD_KILLED;
status = exit_code & 0x7f;
}
- return wait_noreap_copyout(p, pid, uid, why,
- status, infop, ru);
+ return wait_noreap_copyout(wopts, p, pid, uid, why, status);
}
/*
@@ -1250,11 +1260,14 @@ static int wait_task_zombie(struct task_
*/
read_unlock(&tasklist_lock);
- retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0;
+ retval = wopts->wrusage
+ ? getrusage(p, RUSAGE_BOTH, wopts->wrusage) : 0;
status = (p->signal->flags & SIGNAL_GROUP_EXIT)
? p->signal->group_exit_code : p->exit_code;
- if (!retval && stat_addr)
- retval = put_user(status, stat_addr);
+ if (!retval && wopts->wstat)
+ retval = put_user(status, wopts->wstat);
+
+ infop = wopts->winfo;
if (!retval && infop)
retval = put_user(SIGCHLD, &infop->si_signo);
if (!retval && infop)
@@ -1322,16 +1335,16 @@ static int *task_stopped_code(struct tas
* the lock and this task is uninteresting. If we return nonzero, we have
* released the lock and the system call should return.
*/
-static int wait_task_stopped(int ptrace, struct task_struct *p,
- int options, struct siginfo __user *infop,
- int __user *stat_addr, struct rusage __user *ru)
+static int wait_task_stopped(struct wait_opts *wopts,
+ int ptrace, struct task_struct *p)
{
+ struct siginfo __user *infop;
int retval, exit_code, *p_code, why;
uid_t uid = 0; /* unneeded, required by compiler */
pid_t pid;
/* Traditionally we see ptrace'd stopped tasks regardless of options */
- if (!ptrace && !(options & WUNTRACED))
+ if (!ptrace && !(wopts->wflags & WUNTRACED))
return 0;
exit_code = 0;
@@ -1345,7 +1358,7 @@ static int wait_task_stopped(int ptrace,
if (!exit_code)
goto unlock_sig;
- if (!unlikely(options & WNOWAIT))
+ if (!unlikely(wopts->wflags & WNOWAIT))
*p_code = 0;
/* don't need the RCU readlock here as we're holding a spinlock */
@@ -1367,14 +1380,15 @@ unlock_sig:
why = ptrace ? CLD_TRAPPED : CLD_STOPPED;
read_unlock(&tasklist_lock);
- if (unlikely(options & WNOWAIT))
- return wait_noreap_copyout(p, pid, uid,
- why, exit_code,
- infop, ru);
+ if (unlikely(wopts->wflags & WNOWAIT))
+ return wait_noreap_copyout(wopts, p, pid, uid, why, exit_code);
- retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0;
- if (!retval && stat_addr)
- retval = put_user((exit_code << 8) | 0x7f, stat_addr);
+ retval = wopts->wrusage
+ ? getrusage(p, RUSAGE_BOTH, wopts->wrusage) : 0;
+ if (!retval && wopts->wstat)
+ retval = put_user((exit_code << 8) | 0x7f, wopts->wstat);
+
+ infop = wopts->winfo;
if (!retval && infop)
retval = put_user(SIGCHLD, &infop->si_signo);
if (!retval && infop)
@@ -1401,15 +1415,13 @@ unlock_sig:
* the lock and this task is uninteresting. If we return nonzero, we have
* released the lock and the system call should return.
*/
-static int wait_task_continued(struct task_struct *p, int options,
- struct siginfo __user *infop,
- int __user *stat_addr, struct rusage __user *ru)
+static int wait_task_continued(struct wait_opts *wopts, struct task_struct *p)
{
int retval;
pid_t pid;
uid_t uid;
- if (!unlikely(options & WCONTINUED))
+ if (!unlikely(wopts->wflags & WCONTINUED))
return 0;
if (!(p->signal->flags & SIGNAL_STOP_CONTINUED))
@@ -1421,7 +1433,7 @@ static int wait_task_continued(struct ta
spin_unlock_irq(&p->sighand->siglock);
return 0;
}
- if (!unlikely(options & WNOWAIT))
+ if (!unlikely(wopts->wflags & WNOWAIT))
p->signal->flags &= ~SIGNAL_STOP_CONTINUED;
uid = __task_cred(p)->uid;
spin_unlock_irq(&p->sighand->siglock);
@@ -1430,17 +1442,17 @@ static int wait_task_continued(struct ta
get_task_struct(p);
read_unlock(&tasklist_lock);
- if (!infop) {
- retval = ru ? getrusage(p, RUSAGE_BOTH, ru) : 0;
+ if (!wopts->winfo) {
+ retval = wopts->wrusage
+ ? getrusage(p, RUSAGE_BOTH, wopts->wrusage) : 0;
put_task_struct(p);
- if (!retval && stat_addr)
- retval = put_user(0xffff, stat_addr);
+ if (!retval && wopts->wstat)
+ retval = put_user(0xffff, wopts->wstat);
if (!retval)
retval = pid;
} else {
- retval = wait_noreap_copyout(p, pid, uid,
- CLD_CONTINUED, SIGCONT,
- infop, ru);
+ retval = wait_noreap_copyout(wopts, p, pid, uid,
+ CLD_CONTINUED, SIGCONT);
BUG_ON(retval == 0);
}
@@ -1450,19 +1462,16 @@ static int wait_task_continued(struct ta
/*
* Consider @p for a wait by @parent.
*
- * -ECHILD should be in *@notask_error before the first call.
+ * -ECHILD should be in ->notask_error before the first call.
* Returns nonzero for a final return, when we have unlocked tasklist_lock.
* Returns zero if the search for a child should continue;
- * then *@notask_error is 0 if @p is an eligible child,
+ * then ->notask_error is 0 if @p is an eligible child,
* or another error from security_task_wait(), or still -ECHILD.
*/
-static int wait_consider_task(struct task_struct *parent, int ptrace,
- struct task_struct *p, int *notask_error,
- enum pid_type type, struct pid *pid, int options,
- struct siginfo __user *infop,
- int __user *stat_addr, struct rusage __user *ru)
+static int wait_consider_task(struct wait_opts *wopts, struct task_struct *parent,
+ int ptrace, struct task_struct *p)
{
- int ret = eligible_child(type, pid, options, p);
+ int ret = eligible_child(wopts, p);
if (!ret)
return ret;
@@ -1474,8 +1483,8 @@ static int wait_consider_task(struct tas
* to look for security policy problems, rather
* than for mysterious wait bugs.
*/
- if (*notask_error)
- *notask_error = ret;
+ if (wopts->notask_error)
+ wopts->notask_error = ret;
return 0;
}
@@ -1484,7 +1493,7 @@ static int wait_consider_task(struct tas
* This child is hidden by ptrace.
* We aren't allowed to see it now, but eventually we will.
*/
- *notask_error = 0;
+ wopts->notask_error = 0;
return 0;
}
@@ -1495,34 +1504,30 @@ static int wait_consider_task(struct tas
* We don't reap group leaders with subthreads.
*/
if (p->exit_state == EXIT_ZOMBIE && !delay_group_leader(p))
- return wait_task_zombie(p, options, infop, stat_addr, ru);
+ return wait_task_zombie(wopts, p);
/*
* It's stopped or running now, so it might
* later continue, exit, or stop again.
*/
- *notask_error = 0;
+ wopts->notask_error = 0;
if (task_stopped_code(p, ptrace))
- return wait_task_stopped(ptrace, p, options,
- infop, stat_addr, ru);
+ return wait_task_stopped(wopts, ptrace, p);
- return wait_task_continued(p, options, infop, stat_addr, ru);
+ return wait_task_continued(wopts, p);
}
/*
* Do the work of do_wait() for one thread in the group, @tsk.
*
- * -ECHILD should be in *@notask_error before the first call.
+ * -ECHILD should be in ->notask_error before the first call.
* Returns nonzero for a final return, when we have unlocked tasklist_lock.
* Returns zero if the search for a child should continue; then
- * *@notask_error is 0 if there were any eligible children,
+ * ->notask_error is 0 if there were any eligible children,
* or another error from security_task_wait(), or still -ECHILD.
*/
-static int do_wait_thread(struct task_struct *tsk, int *notask_error,
- enum pid_type type, struct pid *pid, int options,
- struct siginfo __user *infop, int __user *stat_addr,
- struct rusage __user *ru)
+static int do_wait_thread(struct wait_opts *wopts, struct task_struct *tsk)
{
struct task_struct *p;
@@ -1531,9 +1536,7 @@ static int do_wait_thread(struct task_st
* Do not consider detached threads.
*/
if (!task_detached(p)) {
- int ret = wait_consider_task(tsk, 0, p, notask_error,
- type, pid, options,
- infop, stat_addr, ru);
+ int ret = wait_consider_task(wopts, tsk, 0, p);
if (ret)
return ret;
}
@@ -1542,17 +1545,12 @@ static int do_wait_thread(struct task_st
return 0;
}
-static int ptrace_do_wait(struct task_struct *tsk, int *notask_error,
- enum pid_type type, struct pid *pid, int options,
- struct siginfo __user *infop, int __user *stat_addr,
- struct rusage __user *ru)
+static int ptrace_do_wait(struct wait_opts *wopts, struct task_struct *tsk)
{
struct task_struct *p;
list_for_each_entry(p, &tsk->ptraced, ptrace_entry) {
- int ret = wait_consider_task(tsk, 1, p, notask_error,
- type, pid, options,
- infop, stat_addr, ru);
+ int ret = wait_consider_task(wopts, tsk, 1, p);
if (ret)
return ret;
}
@@ -1560,38 +1558,36 @@ static int ptrace_do_wait(struct task_st
return 0;
}
-static long do_wait(enum pid_type type, struct pid *pid, int options,
- struct siginfo __user *infop, int __user *stat_addr,
- struct rusage __user *ru)
+static long do_wait(struct wait_opts *wopts)
{
DECLARE_WAITQUEUE(wait, current);
struct task_struct *tsk;
int retval;
- trace_sched_process_wait(pid);
+ trace_sched_process_wait(wopts->wpid);
add_wait_queue(¤t->signal->wait_chldexit,&wait);
repeat:
/*
* If there is nothing that can match our critiera just get out.
- * We will clear @retval to zero if we see any child that might later
- * match our criteria, even if we are not able to reap it yet.
+ * We will clear ->notask_error to zero if we see any child that
+ * might later match our criteria, even if we are not able to reap
+ * it yet.
*/
- retval = -ECHILD;
- if ((type < PIDTYPE_MAX) && (!pid || hlist_empty(&pid->tasks[type])))
+ retval = wopts->notask_error = -ECHILD;
+ if ((wopts->wtype < PIDTYPE_MAX) &&
+ (!wopts->wpid || hlist_empty(&wopts->wpid->tasks[wopts->wtype])))
goto end;
current->state = TASK_INTERRUPTIBLE;
read_lock(&tasklist_lock);
tsk = current;
do {
- int tsk_result = do_wait_thread(tsk, &retval,
- type, pid, options,
- infop, stat_addr, ru);
+ int tsk_result = do_wait_thread(wopts, tsk);
+
if (!tsk_result)
- tsk_result = ptrace_do_wait(tsk, &retval,
- type, pid, options,
- infop, stat_addr, ru);
+ tsk_result = ptrace_do_wait(wopts, tsk);
+
if (tsk_result) {
/*
* tasklist_lock is unlocked and we have a final result.
@@ -1600,25 +1596,27 @@ repeat:
goto end;
}
- if (options & __WNOTHREAD)
+ if (wopts->wflags & __WNOTHREAD)
break;
tsk = next_thread(tsk);
BUG_ON(tsk->signal != current->signal);
} while (tsk != current);
read_unlock(&tasklist_lock);
- if (!retval && !(options & WNOHANG)) {
+ retval = wopts->notask_error;
+ if (!retval && !(wopts->wflags & WNOHANG)) {
retval = -ERESTARTSYS;
if (!signal_pending(current)) {
schedule();
goto repeat;
}
}
-
end:
current->state = TASK_RUNNING;
remove_wait_queue(¤t->signal->wait_chldexit,&wait);
- if (infop) {
+ if (wopts->winfo) {
+ struct siginfo __user *infop = wopts->winfo;
+
if (retval > 0)
retval = 0;
else {
@@ -1647,6 +1645,7 @@ end:
SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
infop, int, options, struct rusage __user *, ru)
{
+ struct wait_opts wopts;
struct pid *pid = NULL;
enum pid_type type;
long ret;
@@ -1676,7 +1675,16 @@ SYSCALL_DEFINE5(waitid, int, which, pid_
if (type < PIDTYPE_MAX)
pid = find_get_pid(upid);
- ret = do_wait(type, pid, options, infop, NULL, ru);
+
+ wopts.wtype = type;
+ wopts.wpid = pid;
+ wopts.wflags = options;
+
+ wopts.winfo = infop;
+ wopts.wstat = NULL;
+ wopts.wrusage = ru;
+
+ ret = do_wait(&wopts);
put_pid(pid);
/* avoid REGPARM breakage on x86: */
@@ -1687,6 +1695,7 @@ SYSCALL_DEFINE5(waitid, int, which, pid_
SYSCALL_DEFINE4(wait4, pid_t, upid, int __user *, stat_addr,
int, options, struct rusage __user *, ru)
{
+ struct wait_opts wopts;
struct pid *pid = NULL;
enum pid_type type;
long ret;
@@ -1708,7 +1717,15 @@ SYSCALL_DEFINE4(wait4, pid_t, upid, int
pid = find_get_pid(upid);
}
- ret = do_wait(type, pid, options | WEXITED, NULL, stat_addr, ru);
+ wopts.wtype = type;
+ wopts.wpid = pid;
+ wopts.wflags = options | WEXITED;
+
+ wopts.winfo = NULL;
+ wopts.wstat = stat_addr;
+ wopts.wrusage = ru;
+
+ ret = do_wait(&wopts);
put_pid(pid);
/* avoid REGPARM breakage on x86: */
next reply other threads:[~2009-05-06 5:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-06 5:33 Oleg Nesterov [this message]
2009-05-06 7:27 ` [FOR REVIEW, PATCH 2/2] introduce "struct wait_opts" to simplify do_wait() pathes Ingo Molnar
2009-05-07 6:41 ` Oleg Nesterov
2009-05-07 7:54 ` Ingo Molnar
2009-05-09 16:15 ` Oleg Nesterov
2009-05-11 10:53 ` Andy Whitcroft
2009-05-11 12:43 ` Ingo Molnar
2009-05-06 20:09 ` Roland McGrath
2009-05-07 6:45 ` Oleg Nesterov
2009-05-07 7:20 ` Roland McGrath
2009-05-07 7:40 ` Oleg Nesterov
2009-05-07 7:49 ` Roland McGrath
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090506053324.GA31988@redhat.com \
--to=oleg@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=roland@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.