Hi,
The majority of the sys_tkill() and sys_tgkill() function code is
duplicated between the two of them. This patch pulls the duplication out
into a separate function -- do_tkill() -- and lets sys_tkill() and
sys_tgkill() be simple wrappers around it. This should make it easier to
maintain in light of future changes.
Diffed against 2.6.14-rc2.
Signed-off-by: Vadim Lobanov <[email protected]>
diff -Npru a/kernel/signal.c b/kernel/signal.c
--- a/kernel/signal.c 2005-09-22 18:32:48.000000000 -0700
+++ b/kernel/signal.c 2005-09-23 18:51:58.000000000 -0700
@@ -2262,26 +2262,13 @@ sys_kill(int pid, int sig)
return kill_something_info(sig, &info, pid);
}
-/**
- * sys_tgkill - send signal to one specific thread
- * @tgid: the thread group ID of the thread
- * @pid: the PID of the thread
- * @sig: signal to be sent
- *
- * This syscall also checks the tgid and returns -ESRCH even if the PID
- * exists but it's not belonging to the target process anymore. This
- * method solves the problem of threads exiting and PIDs getting reused.
- */
-asmlinkage long sys_tgkill(int tgid, int pid, int sig)
+static int do_tkill(int tgid, int pid, int sig)
{
- struct siginfo info;
int error;
+ struct siginfo info;
struct task_struct *p;
- /* This is only valid for single tasks */
- if (pid <= 0 || tgid <= 0)
- return -EINVAL;
-
+ error = -ESRCH;
info.si_signo = sig;
info.si_errno = 0;
info.si_code = SI_TKILL;
@@ -2290,8 +2277,7 @@ asmlinkage long sys_tgkill(int tgid, int
read_lock(&tasklist_lock);
p = find_task_by_pid(pid);
- error = -ESRCH;
- if (p && (p->tgid == tgid)) {
+ if (p && ((tgid <= 0) || (p->tgid == tgid))) {
error = check_kill_permission(sig, &info, p);
/*
* The null signal is a permissions and process existence
@@ -2305,47 +2291,40 @@ asmlinkage long sys_tgkill(int tgid, int
}
}
read_unlock(&tasklist_lock);
+
return error;
}
+/**
+ * sys_tgkill - send signal to one specific thread
+ * @tgid: the thread group ID of the thread
+ * @pid: the PID of the thread
+ * @sig: signal to be sent
+ *
+ * This syscall also checks the tgid and returns -ESRCH even if the PID
+ * exists but it's not belonging to the target process anymore. This
+ * method solves the problem of threads exiting and PIDs getting reused.
+ */
+asmlinkage long sys_tgkill(int tgid, int pid, int sig)
+{
+ /* This is only valid for single tasks */
+ if (pid <= 0 || tgid <= 0)
+ return -EINVAL;
+
+ return (do_tkill(tgid, pid, sig));
+}
+
/*
* Send a signal to only one task, even if it's a CLONE_THREAD task.
*/
asmlinkage long
sys_tkill(int pid, int sig)
{
- struct siginfo info;
- int error;
- struct task_struct *p;
-
/* This is only valid for single tasks */
if (pid <= 0)
return -EINVAL;
- info.si_signo = sig;
- info.si_errno = 0;
- info.si_code = SI_TKILL;
- info.si_pid = current->tgid;
- info.si_uid = current->uid;
-
- read_lock(&tasklist_lock);
- p = find_task_by_pid(pid);
- error = -ESRCH;
- if (p) {
- error = check_kill_permission(sig, &info, p);
- /*
- * The null signal is a permissions and process existence
- * probe. No signal is actually delivered.
- */
- if (!error && sig && p->sighand) {
- spin_lock_irq(&p->sighand->siglock);
- handle_stop_signal(sig, p);
- error = specific_send_sig_info(sig, &info, p);
- spin_unlock_irq(&p->sighand->siglock);
- }
- }
- read_unlock(&tasklist_lock);
- return error;
+ return (do_tkill(0, pid, sig));
}
asmlinkage long
On 9/24/05, Vadim Lobanov <[email protected]> wrote:
> Hi,
>
> The majority of the sys_tkill() and sys_tgkill() function code is
> duplicated between the two of them. This patch pulls the duplication out
> into a separate function -- do_tkill() -- and lets sys_tkill() and
> sys_tgkill() be simple wrappers around it. This should make it easier to
> maintain in light of future changes.
>
A few nitpicks ... :
[snip]
> +static int do_tkill(int tgid, int pid, int sig)
I would probably have made this
static inline int do_tkill(int tgid, int pid, int sig)
[snip]
> + if (p && ((tgid <= 0) || (p->tgid == tgid))) {
Why all the extra parenthesis?
if (p && (tgid <= 0 || p->tgid == tgid)) {
[snip]
> + return (do_tkill(tgid, pid, sig));
return is not a function
return do_tkill(tgid, pid, sig);
[snip]
> + return (do_tkill(0, pid, sig));
again, get rid of the pointless extra parens
return do_tkill(0, pid, sig);
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
I'm having the same problem with 2.6.13, AMD64 X2, Asus A8V Deluxe
motherboard. What's worse is that this is my local net's NTP server, so
it's taking all my other machines' clocks along for the ride, and I'm
losing my associations to the upper strata servers because the skew gets
too great. (So ntpd needs to be restarted periodically.)
I've seen earlier reports on this list about the clock running twice
normal speed. That's not what I'm seeing here; after several hours it's
only ahead by 5 minutes at the moment. (The system has been up 20 days,
but I restarted ntpd a few hours ago, and it resync'd via ntpdate at
that point.) Maybe it would be running at 2X if I kill ntpd, I haven't
checked that.
If it matters, I configured a 250Hz clock tick on this kernel.
--
-- Howard Chu
Chief Architect, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc
OpenLDAP Core Team http://www.openldap.org/project/
On Sat, 24 September 2005 16:52:28 +0200, Jesper Juhl wrote:
>
> [snip]
> > +static int do_tkill(int tgid, int pid, int sig)
>
> I would probably have made this
>
> static inline int do_tkill(int tgid, int pid, int sig)
Why? It would only return the original duplication in binary form and
save a minimal amount of time for something already slow - a system
call. With small caches, the code duplication could even waste more
performance than the missing function call would gain you.
Other nits were well-picked.
J?rn
--
You can't tell where a program is going to spend its time. Bottlenecks
occur in surprising places, so don't try to second guess and put in a
speed hack until you've proven that's where the bottleneck is.
-- Rob Pike
On Sat, 24 Sep 2005, Jesper Juhl wrote:
> On 9/24/05, Vadim Lobanov <[email protected]> wrote:
> > Hi,
> >
> > The majority of the sys_tkill() and sys_tgkill() function code is
> > duplicated between the two of them. This patch pulls the duplication out
> > into a separate function -- do_tkill() -- and lets sys_tkill() and
> > sys_tgkill() be simple wrappers around it. This should make it easier to
> > maintain in light of future changes.
> >
>
> A few nitpicks ... :
>
> [snip]
> > +static int do_tkill(int tgid, int pid, int sig)
>
> I would probably have made this
>
> static inline int do_tkill(int tgid, int pid, int sig)
Probably not necessary to inline this, as per the previous comments.
> [snip]
> > + if (p && ((tgid <= 0) || (p->tgid == tgid))) {
>
> Why all the extra parenthesis?
>
> if (p && (tgid <= 0 || p->tgid == tgid)) {
>
>
> [snip]
> > + return (do_tkill(tgid, pid, sig));
>
> return is not a function
>
> return do_tkill(tgid, pid, sig);
>
> [snip]
> > + return (do_tkill(0, pid, sig));
>
> again, get rid of the pointless extra parens
>
> return do_tkill(0, pid, sig);
These are all no biggie. I'll remove the parens and resend.
> --
> Jesper Juhl <[email protected]>
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please http://www.expita.com/nomime.html
>
Thanks for the comments.
-Vadim Lobanov
On 9/24/05, J?rn Engel <[email protected]> wrote:
> On Sat, 24 September 2005 16:52:28 +0200, Jesper Juhl wrote:
> >
> > [snip]
> > > +static int do_tkill(int tgid, int pid, int sig)
> >
> > I would probably have made this
> >
> > static inline int do_tkill(int tgid, int pid, int sig)
>
> Why? It would only return the original duplication in binary form and
> save a minimal amount of time for something already slow - a system
> call. With small caches, the code duplication could even waste more
> performance than the missing function call would gain you.
>
You are right, I didn't think that through properly.
Thank you for catching that.
> Other nits were well-picked.
>
Thanks.
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html