From: Andrew Morton <akpm@osdl.org>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: torvalds@osdl.org, mingo@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] kthread_create
Date: Tue, 30 Dec 2003 20:49:10 -0800 [thread overview]
Message-ID: <20031230204910.0e767b50.akpm@osdl.org> (raw)
In-Reply-To: <20031231042016.958DC2C04B@lists.samba.org>
Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Hi all,
>
> Ingo read through this before and liked it: this is the basis
> of the Hotplug CPU patch, and as such has been stressed fairly well.
> Tested stand-alone, and included here for wider review.
It would be nice to be able to see all the hotplug CPU patches in one
place, to get a feel for their shape and size. That way, we can decide
whether we need to look at this patch ;)
> D: kthread_create(), kthread_start() and kthread_destroy(). These
A few things:
> +static struct kt_message ktm_receive(void)
> +{
> + struct kt_message m;
> +
> + for (;;) {
> + spin_lock(&ktm_lock);
> + if (ktm.to == current)
> + break;
> + current->state = TASK_INTERRUPTIBLE;
> + spin_unlock(&ktm_lock);
> + schedule();
> + }
If the calling task has a signal pending, this could become a tight loop?
> +
> +static int kthread(void *data)
> +{
> + /* Copy data: it's on keventd_init's stack */
> + struct kthread k = *(struct kthread *)data;
> + struct kt_message m;
> + int ret = 0;
> + sigset_t blocked;
> +
> + strcpy(current->comm, k.name);
> +
> + /* Block and flush all signals. */
> + sigfillset(&blocked);
> + sigprocmask(SIG_BLOCK, &blocked, NULL);
> + flush_signals(current);
> +
deamonize() was not suitable here?
> + /* Send to spawn_kthread, so it knows who we are. */
> + ktm_send(ktm.info, current);
> +
> + /* Receive from kthread_start or kthread_destroy */
> + m = ktm_receive();
> + if (!m.info)
> + goto stop;
> + if (k.initfn && (ret = k.initfn(k.data)) < 0)
> + goto stop;
> + ktm_send(m.from, current);
> +
> + for (;;) {
> + if (time_to_die(&m))
> + break;
> +
> + /* If it fails, just wait until kthread_destroy. */
> + if (k.corefn && (ret = k.corefn(k.data)) < 0)
> + k.corefn = NULL;
> +
> + if (time_to_die(&m))
> + break;
> +
> + schedule();
> + }
In what state is this schedule() called? If it's TASK_RUNNING (or
TASK_INTERRUPTIBLE with signal_pending()) and this task has rt priority
higher than the thing it is waiting for we could have a problem?
> +
> + current->state = TASK_RUNNING;
> +stop:
> + ktm_send(m.from, ERR_PTR(ret));
> + return ret;
> +}
> +
> +struct kthread_create
> +{
> + struct task_struct *result;
> + struct kthread k;
> + struct completion done;
> +};
> +
`kthread_create' sounds like the name of a function to me, not a structure.
> +struct task_struct *kthread_create(int (*initfn)(void *data),
I was right! ;)
It would be nice to kerneldocify kthread_create(), kthread_start() and
kthread_destroy() sometime.
> +static void wait_for_death(struct task_struct *k)
> +{
> + while (!(k->state & TASK_ZOMBIE) && !(k->state & TASK_DEAD))
> + yield();
> +}
> +
If the calling task has higher rt priority than *k, could this not become a
busy loop? It would be preferable to use a real sleep/wait primitive here.
next prev parent reply other threads:[~2003-12-31 4:48 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-12-31 3:31 [PATCH 1/2] kthread_create Rusty Russell
2003-12-31 4:33 ` Jeff Garzik
2003-12-31 5:28 ` Rusty Russell
2003-12-31 6:34 ` Jeff Garzik
2003-12-31 8:47 ` Benjamin Herrenschmidt
2004-01-01 23:51 ` Rusty Russell
2003-12-31 4:49 ` Andrew Morton [this message]
2003-12-31 5:18 ` Rusty Russell
2003-12-31 5:06 ` Davide Libenzi
2003-12-31 5:34 ` Rusty Russell
2003-12-31 5:56 ` Davide Libenzi
2003-12-31 6:27 ` Rusty Russell
2004-01-01 3:45 ` Davide Libenzi
2004-01-02 7:09 ` Rusty Russell
2004-01-02 16:58 ` Davide Libenzi
2004-01-03 3:05 ` Rusty Russell
2004-01-03 3:43 ` Davide Libenzi
2004-01-03 11:47 ` Rusty Russell
2004-01-04 4:23 ` Davide Libenzi
2004-03-29 15:42 ` Davide Libenzi
2004-01-03 19:00 ` Davide Libenzi
2004-01-03 23:53 ` Davide Libenzi
2004-01-04 2:34 ` Rusty Russell
2004-01-04 4:42 ` Davide Libenzi
2004-01-04 4:55 ` Davide Libenzi
2004-01-04 9:35 ` Rusty Russell
2004-01-04 23:03 ` Davide Libenzi
2004-01-05 4:09 ` Rusty Russell
2004-01-05 5:06 ` Davide Libenzi
2004-01-05 6:38 ` Rusty Russell
2004-01-05 6:52 ` Davide Libenzi
2004-01-07 7:00 ` Rusty Russell
2004-01-07 7:25 ` Davide Libenzi
2004-01-08 0:33 ` Rusty Russell
2004-03-29 15:42 ` Davide Libenzi
2004-03-29 15:42 ` Davide Libenzi
2004-03-29 15:41 ` Davide Libenzi
2004-03-29 15:42 ` Rusty Russell
2004-03-29 15:40 ` Davide Libenzi
2004-03-29 15:41 ` Rusty Russell
2004-03-29 15:39 ` Davide Libenzi
2004-03-29 15:39 ` Rusty Russell
2004-03-29 15:38 ` Davide Libenzi
2003-12-31 6:31 ` Srivatsa Vaddagiri
2003-12-31 7:12 ` Davide Libenzi
2003-12-31 23:25 ` Rusty Russell
2003-12-31 18:02 Albert Cahalan
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=20031230204910.0e767b50.akpm@osdl.org \
--to=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rusty@rustcorp.com.au \
--cc=torvalds@osdl.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).