Home » Mailing lists » Devel » Remaining straight forward kthread API conversions...
Re: [PATCH] cpci_hotplug: Convert to use the kthread API [message #18582 is a reply to message #18581] |
Mon, 07 May 2007 18:18 |
Scott Murray
Messages: 4 Registered: April 2007
|
Junior Member |
|
|
On Fri, 4 May 2007, Christoph Hellwig wrote:
> On Fri, Apr 27, 2007 at 06:07:49PM -0400, Scott Murray wrote:
> > Sorry, it took me a few days to get to testing this out. It looks good,
> > but I had to make a couple of tweaks to avoid a hang when rmmod'ing a
> > board driver. The board drivers do:
> >
> > cpci_hp_stop()
> > cpci_hp_unregister_controller(controller)
> >
> > to shutdown, and the check in cpci_hp_unregister_controller if the thread
> > is running wasn't working due to a bit too much code being excised. The
> > result was kthread_stop being called twice, which hangs. I've indicated
> > my changes to avoid this inline below.
>
> Can you forward the patches with your fix to Andrew to make sure he
> picks it up?
Andrew, here is my updated version of Christoph's kthread conversion
patch for cpci_hotplug. I've CC'ed Kristen so she won't be surprised
when this eventually goes to mainline.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Scott Murray <scottm@somanetworks.com>
---
diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
index 6845515..ed4d44e 100644
--- a/drivers/pci/hotplug/cpci_hotplug_core.c
+++ b/drivers/pci/hotplug/cpci_hotplug_core.c
@@ -35,6 +35,7 @@
#include <linux/smp_lock.h>
#include <asm/atomic.h>
#include <linux/delay.h>
+#include <linux/kthread.h>
#include "cpci_hotplug.h"
#define DRIVER_AUTHOR "Scott Murray <scottm@somanetworks.com>"
@@ -59,9 +60,8 @@ static int slots;
static atomic_t extracting;
int cpci_debug;
static struct cpci_hp_controller *controller;
-static struct semaphore event_semaphore; /* mutex for process loop (up if something to process) */
-static struct semaphore thread_exit; /* guard ensure thread has exited before calling it quits */
-static int thread_finished = 1;
+static struct task_struct *cpci_thread;
+static int thread_finished;
static int enable_slot(struct hotplug_slot *slot);
static int disable_slot(struct hotplug_slot *slot);
@@ -357,9 +357,7 @@ cpci_hp_intr(int irq, void *data)
controller->ops->disable_irq();
/* Trigger processing by the event thread */
- dbg("Signal event_semaphore");
- up(&event_semaphore);
- dbg("exited cpci_hp_intr");
+ wake_up_process(cpci_thread);
return IRQ_HANDLED;
}
@@ -521,17 +519,12 @@ event_thread(void *data)
{
int rc;
- lock_kernel();
- daemonize("cpci_hp_eventd");
- unlock_kernel();
-
dbg("%s - event thread started", __FUNCTION__);
while (1) {
dbg("event thread sleeping");
- down_interruptible(&event_semaphore);
- dbg("event thread woken, thread_finished = %d",
- thread_finished);
- if (thread_finished || signal_pending(current))
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ if (kthread_should_stop())
break;
do {
rc = check_slots();
@@ -541,18 +534,17 @@ event_thread(void *data)
} else if (rc < 0) {
dbg("%s - error checking slots", __FUNCTION__);
thread_finished = 1;
- break;
+ goto out;
}
- } while (atomic_read(&extracting) && !thread_finished);
- if (thread_finished)
+ } while (atomic_read(&extracting) && !kthread_should_stop());
+ if (kthread_should_stop())
break;
/* Re-enable ENUM# interrupt */
dbg("%s - re-enabling irq", __FUNCTION__);
controller->ops->enable_irq();
}
- dbg("%s - event thread signals exit", __FUNCTION__);
- up(&thread_exit);
+ out:
return 0;
}
@@ -562,12 +554,8 @@ poll_thread(void *data)
{
int rc;
- lock_kernel();
- daemonize("cpci_hp_polld");
- unlock_kernel();
-
while (1) {
- if (thread_finished || signal_pending(current))
+ if (kthread_should_stop() || signal_pending(current))
break;
if (controller->ops->query_enum()) {
do {
@@ -578,48 +566,36 @@ poll_thread(void *data)
} else if (rc < 0) {
dbg("%s - error checking slots", __FUNCTION__);
thread_finished = 1;
- break;
+ goto out;
}
- } while (atomic_read(&extracting) && !thread_finished);
+ } while (atomic_read(&extracting) && !kthread_should_stop());
}
msleep(100);
}
- dbg("poll thread signals exit");
- up(&thread_exit);
+ out:
return 0;
}
static int
cpci_start_thread(void)
{
- int pid;
-
- /* initialize our semaphores */
- init_MUTEX_LOCKED(&event_semaphore);
- init_MUTEX_LOCKED(&thread_exit);
- thread_finished = 0;
-
if (controller->irq)
- pid = kernel_thread(event_thread, NULL, 0);
+ cpci_thread = kthread_run(event_thread, NULL, "cpci_hp_eventd");
else
- pid = kernel_thread(poll_thread, NULL, 0);
- if (pid < 0) {
+ cpci_thread = kthread_run(poll_thread, NULL, "cpci_hp_polld");
+ if (IS_ERR(cpci_thread)) {
err("Can't start up our thread");
- return -1;
+ return PTR_ERR(cpci_thread);
}
- dbg("Our thread pid = %d", pid);
+ thread_finished = 0;
return 0;
}
static void
cpci_stop_thread(void)
{
+ kthread_stop(cpci_thread);
thread_finished = 1;
- dbg("thread finish command given");
- if (controller->irq)
- up(&event_semaphore);
- dbg("wait for thread to exit");
- down(&thread_exit);
}
int
--
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.com
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] cpci_hotplug: Convert to use the kthread API [message #18585 is a reply to message #18582] |
Wed, 09 May 2007 23:24 |
akpm
Messages: 224 Registered: March 2007
|
Senior Member |
|
|
On Mon, 7 May 2007 14:18:29 -0400 (EDT)
Scott Murray <scottm@somanetworks.com> wrote:
> On Fri, 4 May 2007, Christoph Hellwig wrote:
>
> > On Fri, Apr 27, 2007 at 06:07:49PM -0400, Scott Murray wrote:
> > > Sorry, it took me a few days to get to testing this out. It looks good,
> > > but I had to make a couple of tweaks to avoid a hang when rmmod'ing a
> > > board driver. The board drivers do:
> > >
> > > cpci_hp_stop()
> > > cpci_hp_unregister_controller(controller)
> > >
> > > to shutdown, and the check in cpci_hp_unregister_controller if the thread
> > > is running wasn't working due to a bit too much code being excised. The
> > > result was kthread_stop being called twice, which hangs. I've indicated
> > > my changes to avoid this inline below.
> >
> > Can you forward the patches with your fix to Andrew to make sure he
> > picks it up?
>
> Andrew, here is my updated version of Christoph's kthread conversion
> patch for cpci_hotplug. I've CC'ed Kristen so she won't be surprised
> when this eventually goes to mainline.
A patch in this area would normally go
you->kristen->mainline
|
v
-mm
or
you->kristen->greg->mainline
|
v
-mm
or
you->me->greg->mainline (gets an Acked-by somewhere)
|
v
-mm
or
you->kristen->Len->mainline
|
v
-mm
or something else.
Kristen, how do you want to play this?
Do you run a tree? If so, lemmeatit ;)
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] cpci_hotplug: Convert to use the kthread API [message #18586 is a reply to message #18585] |
Thu, 10 May 2007 00:00 |
Kristen Carlson Accar
Messages: 1 Registered: May 2007
|
Junior Member |
|
|
On Wed, 9 May 2007 16:24:30 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon, 7 May 2007 14:18:29 -0400 (EDT)
> Scott Murray <scottm@somanetworks.com> wrote:
>
> > On Fri, 4 May 2007, Christoph Hellwig wrote:
> >
> > > On Fri, Apr 27, 2007 at 06:07:49PM -0400, Scott Murray wrote:
> > > > Sorry, it took me a few days to get to testing this out. It looks good,
> > > > but I had to make a couple of tweaks to avoid a hang when rmmod'ing a
> > > > board driver. The board drivers do:
> > > >
> > > > cpci_hp_stop()
> > > > cpci_hp_unregister_controller(controller)
> > > >
> > > > to shutdown, and the check in cpci_hp_unregister_controller if the thread
> > > > is running wasn't working due to a bit too much code being excised. The
> > > > result was kthread_stop being called twice, which hangs. I've indicated
> > > > my changes to avoid this inline below.
> > >
> > > Can you forward the patches with your fix to Andrew to make sure he
> > > picks it up?
> >
> > Andrew, here is my updated version of Christoph's kthread conversion
> > patch for cpci_hotplug. I've CC'ed Kristen so she won't be surprised
> > when this eventually goes to mainline.
>
> A patch in this area would normally go
>
> you->kristen->mainline
> |
> v
> -mm
>
> or
>
> you->kristen->greg->mainline
> |
> v
> -mm
>
> or
>
> you->me->greg->mainline (gets an Acked-by somewhere)
> |
> v
> -mm
>
> or
>
> you->kristen->Len->mainline
> |
> v
> -mm
>
> or something else.
>
> Kristen, how do you want to play this?
>
> Do you run a tree? If so, lemmeatit ;)
>
So, we (Greg and I) have talked about this before - I do have a tree,
but I normally send things to Greg rather than directly to you. I should
probably change that (why add levels of indirection...), but for the
immediate purpose of getting this patch tested etc just go ahead and take
it in and when I get my act together I will put it in my tree.
Thanks,
Kristen
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
Re: [PATCH] cpci_hotplug: Convert to use the kthread API [message #18588 is a reply to message #18585] |
Thu, 10 May 2007 18:29 |
Scott Murray
Messages: 4 Registered: April 2007
|
Junior Member |
|
|
On Wed, 9 May 2007, Andrew Morton wrote:
> On Mon, 7 May 2007 14:18:29 -0400 (EDT)
> Scott Murray <scottm@somanetworks.com> wrote:
>
> > On Fri, 4 May 2007, Christoph Hellwig wrote:
> >
> > > On Fri, Apr 27, 2007 at 06:07:49PM -0400, Scott Murray wrote:
> > > > Sorry, it took me a few days to get to testing this out. It looks good,
> > > > but I had to make a couple of tweaks to avoid a hang when rmmod'ing a
> > > > board driver. The board drivers do:
> > > >
> > > > cpci_hp_stop()
> > > > cpci_hp_unregister_controller(controller)
> > > >
> > > > to shutdown, and the check in cpci_hp_unregister_controller if the thread
> > > > is running wasn't working due to a bit too much code being excised. The
> > > > result was kthread_stop being called twice, which hangs. I've indicated
> > > > my changes to avoid this inline below.
> > >
> > > Can you forward the patches with your fix to Andrew to make sure he
> > > picks it up?
> >
> > Andrew, here is my updated version of Christoph's kthread conversion
> > patch for cpci_hotplug. I've CC'ed Kristen so she won't be surprised
> > when this eventually goes to mainline.
>
> A patch in this area would normally go
>
> you->kristen->mainline
> |
> v
> -mm
>
> or
>
> you->kristen->greg->mainline
> |
> v
> -mm
>
> or
>
> you->me->greg->mainline (gets an Acked-by somewhere)
> |
> v
> -mm
>
> or
>
> you->kristen->Len->mainline
> |
> v
> -mm
>
> or something else.
My apologies, it wasn't entirely clear to me that that was happening with
the other kthread conversion patches in the big batch, so I didn't follow
the normal procedure. I'll try to stick to the regular flow up the chain
going forward.
Scott
--
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.com
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] ia64 sn xpc: Convert to use kthread API. [message #18603 is a reply to message #18576] |
Thu, 17 May 2007 13:44 |
Dean Nelson
Messages: 6 Registered: April 2007
|
Junior Member |
|
|
On Wed, May 02, 2007 at 09:44:11AM -0600, Eric W. Biederman wrote:
> Dean Nelson <dcn@sgi.com> writes:
> > On Thu, Apr 26, 2007 at 01:11:15PM -0600, Eric W. Biederman wrote:
> >>
> >> Ok. Because of the module unloading issue, and because we don't have
> >> a lot of these threads running around, the current plan is to fix
> >> thread_create and kthread_stop so that they must always be paired,
> >> and so that kthread_stop will work correctly if the task has already
> >> exited.
> >>
> >> Basically that just involves calling get_task_struct in kthread_create
> >> and put_task_struct in kthread_stop.
> >
> > Okay, so I need to expand upon Christoph Hellwig's patch so that all
> > the kthread_create()'d threads are kthread_stop()'d.
> >
> > This is easy to do for the XPC thread that exists for the lifetime of XPC,
> > as well as for the threads created to manage the SGI system partitions.
> >
> > XPC has the one discovery thread that is created when XPC is first started
> > and exits as soon as it has finished discovering all existing SGI system
> > partitions. With your forthcoming change to kthread_stop() that will allow
> > it to be called after the thread has exited, doing this one is also easy.
> > Note that the kthread_stop() for this discovery thread won't occur until
> > XPC is rmmod'd. This means that its task_struct will not be freed for
> > possibly a very long time (i.e., weeks). Is that a problem?
>
> As long as there is only one, not really. It would be good if we could
> get rid of it though.
>
> The practical problem is the race with rmmod, in particular if someone
> calls rmmod while this thread is still running.
I guess I'm not seeing the race with rmmod that you're talking
about? In XPC's case, rmmod calls xpc_exit() which currently does a
wait_for_completion() on the discovery thread and on the other thread
mentioned above. These will be changed to kthread_stop() calls. And if
the discovery thread has already exited the kthread_stop() will return
immediately and if not it will wait until the discovery thread has
exited. rmmod won't return from xpc_exit() until both threads have exited.
Any thought as to when the changes to kthread_stop() that allow it to be
called for a kthread that has already exited will get into the -mm tree?
> > Is there any way to have a version of kthread_create() that doesn't
> > require a matching kthread_stop()? Or add a kthread_not_stopping()
> > that does the put_task_struct() call, so as to eliminate the need for
> > calling kthread_stop()?
>
> Yes. I was thinking calling it kthread_orphan or something like that.
> We can't make anything like that the default, because of the modular
> remove problem, but it's not to hard.
Again, when xpc_exit() is called by rmmod it waits for XPC's pool of
threads to exit before it returns, so not a problem.
Any thought as to when kthread_orphan() will get into the -mm tree? Once
kthread_stop() is changed and kthread_orphan() added I can proceed with
a patch to change XPC to use the kthread API.
> > Or should we reconsider the kthread pool approach
> > (and get XPC out of the thread management business altogether)? Robin
> > Holt is putting together a proposal for how one could do a kthread pool,
> > it should provide a bit more justification for going down that road.
Robin has changed his mind about tieing in the management of a pool
of threads with the kthread API, so there won't be the fore mentioned
proposal.
Thanks,
Dean
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Goto Forum:
Current Time: Fri Oct 18 18:48:14 GMT 2024
Total time taken to generate the page: 0.05989 seconds
|