Home » Mailing lists » Devel » Remaining straight forward kthread API conversions...
|
|
Re: [PATCH] nfs lockd reclaimer: Convert to kthread API [message #18442 is a reply to message #18391] |
Sat, 21 April 2007 19:47 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
ebiederm
Messages: 1354 Registered: February 2006
|
Senior Member |
|
|
Trond Myklebust <trond.myklebust@fys.uio.no> writes:
> On Thu, 2007-04-19 at 14:40 -0700, Andrew Morton wrote:
>> Using signals to communicate with kernel threads is fairly unpleasant, IMO.
>> We have much simpler, faster and more idiomatic ways of communicating
>> between threads in-kernel and there are better ways in which userspace can
>> communicate with the kernel - system calls, for example...
>>
>> So I think generally any move which gets us away from using signals in
>> kernel threads is moving in a good direction.
>
> I have yet to see a proposal which did. Eric's patch was eliminating
> signals in kernel threads that used them without proposing any
> replacement mechanism or showing that he had plans to do so. That is a
> good reason for a veto.
Possibly I just hadn't looked close enough. The signals looked like
a redundant mechanism.
>> > > With pid namespaces all kernel threads will disappear so how do
>> > > we cope with the problem when the sysadmin can not see the kernel
>> > > threads?
>> >
>> > Then you have a usability problem. How does the sysadmin reboot the
>> > system if there is no way to shut down the processes that are hanging on
>> > an unresponsive filesystem?
>>
>> Where's the hang? A user process is stuck on h_rwsem?
>>
>> If so, would it be appropriate to convert the user process to use
>> down_foo_interruptible(), so that the operator can just kill the user
>> process as expected, rather than having to futz around killing kernel
>> threads?
>
> If an NFS server reboots, then the locks held by user processes on the
> client need to be re-established by when it comes up again. Otherwise,
> the processes that thought they were holding locks will suddenly fail.
> This recovery job is currently the done by a kernel thread.
>
> The question is then what to do if the server crashes again while the
> kernel thread is re-establishing the locks. Particularly if it never
> comes back again.
> Currently, the administrator can intervene by killing anything that has
> open files on that volume and kill the recovery kernel thread.
> You'll also note that lockd_down(), nfsd_down() etc all use signals to
> inform lockd(), nfsd() etc that they should be shutting down. Since the
> reclaimer thread is started by the lockd() thread using CLONE_SIGHAND,
> this means that we also automatically kill any lingering recovery
> threads whenever we shutdown lockd().
Maybe I'm missing something but I think you are referring to the semantics
of do_group_exit in the presence of CLONE_THREAD. All sharing a
sighand should do is cause the sharing of the signal handler. Causing
allow_signal and disallow_signal to act on a group of threads instead
of a single thread. I don't recall clone_sighand having any
other effects.
> These mechanisms need to be replaced _before_ we start shooting down
> sigallow() etc in the kernel.
Reasonable if these mechanisms are not redundant.
Thinking it through because everything having to do with nfs mounting and
unmounting is behind the privileged mount operation this is not going to
become an issue until we start allowing unprivileged nfs mounts. Because
we cannot delegate control of nfs mount and unmount operations until then.
Since signals do not pose a immediate barrier to forward progress like
daemonize and kernel_thread we can leave things as is until we can
sort this out.
Eric
_______________________________________________
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 #18463 is a reply to message #18284] |
Sun, 22 April 2007 12:05 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 12:55:29AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
>
> kthread_run replaces the kernel_thread and daemonize calls
> during thread startup.
>
> Calls to signal_pending were also removed as it is currently
> impossible for the cpci_hotplug thread to receive signals.
This drivers thread are a bit of a miss, although a lot better than
most other pci hotplug drivers :)
Below is more complete conversion to the kthread infrastructure +
wake_up_process to wake the thread. Note that we had to keep
a thread_finished variable because the existing one had dual use.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/pci/hotplug/cpci_hotplug_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/cpci_hotplug_core.c 2007-04-22 12:54:17.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/cpci_hotplug_core.c 2007-04-22 13:01:42.000000000 +0200
@@ -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,34 @@ 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);
return 0;
}
static void
cpci_stop_thread(void)
{
- thread_finished = 1;
- dbg("thread finish command given");
- if (controller->irq)
- up(&event_semaphore);
- dbg("wait for thread to exit");
- down(&thread_exit);
+ kthread_stop(cpci_thread);
}
int
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] ibmphp: Convert to use the kthreads API [message #18464 is a reply to message #18285] |
Sun, 22 April 2007 12:09 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 12:55:30AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
>
> kthread_run replaces kernel_thread and dameonize.
>
> allow_signal is unnecessary and has been removed.
> tid_poll was unused and has been removed.
Thread handling in this driver is quite interesting. Greg has his name
in there, so we can blame everything on him ;-)
Below is my take at cleaning at least the thread-related bits up:
- full switch to kthread infrastructure
- switch semOperations to a mutex, and give it a proper name
- remove the useless hpc_poll_thread wrapper
- remove ibmphp_hpc_initvars - everything left can easily be
initialized statically
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/pci/hotplug/ibmphp.h
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/ibmphp.h 2007-04-22 12:44:04.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/ibmphp.h 2007-04-22 12:44:07.000000000 +0200
@@ -392,7 +392,6 @@ extern int ibmphp_add_pfmem_from_mem (st
extern struct bus_node *ibmphp_find_res_bus (u8);
extern void ibmphp_print_test (void); /* for debugging purposes */
-extern void ibmphp_hpc_initvars (void);
extern int ibmphp_hpc_readslot (struct slot *, u8, u8 *);
extern int ibmphp_hpc_writeslot (struct slot *, u8);
extern void ibmphp_lock_operations (void);
Index: linux-2.6/drivers/pci/hotplug/ibmphp_core.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/ibmphp_core.c 2007-04-22 12:44:11.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/ibmphp_core.c 2007-04-22 12:44:16.000000000 +0200
@@ -1368,8 +1368,6 @@ static int __init ibmphp_init(void)
ibmphp_debug = debug;
- ibmphp_hpc_initvars();
-
for (i = 0; i < 16; i++)
irqs[i] = 0;
Index: linux-2.6/drivers/pci/hotplug/ibmphp_hpc.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/ibmphp_hpc.c 2007-04-22 12:31:23.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/ibmphp_hpc.c 2007-04-22 12:45:51.000000000 +0200
@@ -35,6 +35,7 @@
#include <linux/smp_lock.h>
#include <linux/init.h>
#include <linux/mutex.h>
+#include <linux/kthread.h>
#include "ibmphp.h"
@@ -101,12 +102,10 @@ static int to_debug = 0;
//----------------------------------------------------------------------------
// global variables
//----------------------------------------------------------------------------
-static int ibmphp_shutdown;
-static int tid_poll;
-static struct mutex sem_hpcaccess; // lock access to HPC
-static struct semaphore semOperations; // lock all operations and
+static struct task_struct *ibmphp_poll_thread;
+static DEFINE_MUTEX(sem_hpcaccess); // lock access to HPC
+static DEFINE_MUTEX(ibmphp_op_sem); // lock all operations and
// access to data structures
-static struct semaphore sem_exit; // make sure polling thread goes away
//----------------------------------------------------------------------------
// local function prototypes
//----------------------------------------------------------------------------
@@ -116,33 +115,11 @@ static u8 hpc_writecmdtoindex (u8, u8);
static u8 hpc_readcmdtoindex (u8, u8);
static void get_hpc_access (void);
static void free_hpc_access (void);
-static void poll_hpc (void);
static int process_changeinstatus (struct slot *, struct slot *);
static int process_changeinlatch (u8, u8, struct controller *);
-static int hpc_poll_thread (void *);
static int hpc_wait_ctlr_notworking (int, struct controller *, void __iomem *, u8 *);
//----------------------------------------------------------------------------
-
-/*----------------------------------------------------------------------
-* Name: ibmphp_hpc_initvars
-*
-* Action: initialize semaphores and variables
-*---------------------------------------------------------------------*/
-void __init ibmphp_hpc_initvars (void)
-{
- debug ("%s - Entry\n", __FUNCTION__);
-
- mutex_init(&sem_hpcaccess);
- init_MUTEX (&semOperations);
- init_MUTEX_LOCKED (&sem_exit);
- to_debug = 0;
- ibmphp_shutdown = 0;
- tid_poll = 0;
-
- debug ("%s - Exit\n", __FUNCTION__);
-}
-
/*----------------------------------------------------------------------
* Name: i2c_ctrl_read
*
@@ -798,7 +775,7 @@ void free_hpc_access (void)
*---------------------------------------------------------------------*/
void ibmphp_lock_operations (void)
{
- down (&semOperations);
+ mutex_lock(&ibmphp_op_sem);
to_debug = 1;
}
@@ -808,7 +785,7 @@ void ibmphp_lock_operations (void)
void ibmphp_unlock_operations (void)
{
debug ("%s - Entry\n", __FUNCTION__);
- up (&semOperations);
+ mutex_unlock(&ibmphp_op_sem);
to_debug = 0;
debug ("%s - Exit\n", __FUNCTION__);
}
@@ -819,7 +796,7 @@ void ibmphp_unlock_operations (void)
#define POLL_LATCH_REGISTER 0
#define POLL_SLOTS 1
#define POLL_SLEEP 2
-static void poll_hpc (void)
+static int poll_hpc(void *data)
{
struct slot myslot;
struct slot *pslot = NULL;
@@ -833,12 +810,9 @@ static void poll_hpc (void)
debug ("%s - Entry\n", __FUNCTION__);
- while (!ibmphp_shutdown) {
- if (ibmphp_shutdown)
- break;
-
+ while (!kthread_should_stop()) {
/* try to get the lock to do some kind of hardware access */
- down (&semOperations);
+ mutex_lock(&ibmphp_op_sem);
switch (poll_state) {
case POLL_LATCH_REGISTER:
@@ -893,14 +867,13 @@ static void poll_hpc (void)
break;
case POLL_SLEEP:
/* don't sleep with a lock on the hardware */
- up (&semOperations);
+ mutex_unlock(&ibmphp_op_sem);
msleep(POLL_INTERVAL_SEC * 1000);
- if (ibmphp_shutdown)
+ if (kthread_should_stop())
break;
- down (&semOperations);
-
+ mutex_lock(&ibmphp_op_sem);
if (poll_count >= POLL_LATCH_CNT) {
poll_count = 0;
poll_state = POLL_SLOTS;
@@ -909,12 +882,13 @@ static void poll_hpc (void)
break;
}
/* give up the hardware semaphore */
- up (&semOperations);
+ mutex_unlock(&ibmphp_op_sem);
/* sleep for a short time just for good measure */
msleep(100);
}
- up (&sem_exit);
debug ("%s - Exit\n", __FUNCTION__);
+
+ return 0;
}
@@ -1050,47 +1024,19 @@ static int process_changeinlatch (u8 old
}
/*----------------------------------------------------------------------
-* Name: hpc_poll_thread
-*
-* Action: polling
-*
-* Return 0
-* Value:
-*---------------------------------------------------------------------*/
-static int hpc_poll_thread (void *data)
-{
- debug ("%s - Entry\n", __FUNCTION__);
-
- daemonize("hpc_poll");
- allow_signal(SIGKILL);
-
- poll_hpc ();
-
- tid_poll = 0;
- debug ("%s - Exit\n", __FUNCTION__);
- return 0;
-}
-
-
-/*----------------------------------------------------------------------
* Name: ibmphp_hpc_start_poll_thread
*
* Action: start polling thread
*---------------------------------------------------------------------*/
int __init ibmphp_hpc_start_poll_thread (void)
{
- int rc = 0;
-
- debug ("%s - Entry\n", __FUNCTION__);
-
- tid_poll = kernel_thread (hpc_poll_thread, NULL, 0);
- if (tid_poll < 0) {
+ ibmphp_poll_thread = kthread_run(poll_hpc, NULL, "hpc_poll");
+ if (IS_ERR(ibmphp_poll_thread)) {
err ("%s - Error, thread not started\n", __FUNCTION__);
- rc = -1;
+ return PTR_ERR(ibmphp_poll_thread);
}
- debug ("%s - Exit tid_poll[%d] rc[%d]\n", __FUNCTION__, tid_poll, rc);
- return rc;
+ return 0;
}
/*----------------------------------------------------------------------
@@ -1100,28 +1046,11 @@ int __init ibmphp_hpc_start_poll_thread
*---------------------------------------------------------------------*/
void __exit ibmphp_hpc_stop_poll_thread (void)
{
- debug ("%s - Entry\n", __FUNCTION__);
+ kthread_stop(ibmphp_poll_thread);
- ibmphp_shutdown = 1;
- debug ("before locking operations \n");
ibmphp_lock_operations ();
- debug ("after locking operations \n");
-
- // wait for poll thread to exit
- debug ("before sem_exit down \n");
- down (&sem_exit);
- debug ("after sem_exit down \n");
-
- // cleanup
- debug ("before free_hpc_access \n");
free_hpc_access ();
- debug ("after free_hpc_access \n");
ibmphp_unlock_operations ();
- debug ("after unlock operations \n");
- up (&sem_exit);
- debug ("after sem exit up\n");
-
- debug ("%s - Exit\n", __FUNCTION__);
}
/*----------------------------------------------------------------------
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] cpqphp: Convert to use the kthread API [message #18465 is a reply to message #18286] |
Sun, 22 April 2007 12:12 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 12:55:31AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
>
> This patch changes cpqphp to use kthread_run and not
> kernel_thread and daemonize to startup and setup
> the cpqphp thread.
Thread handling in this driver (and actually everything else) seems
to be written by a crackmonkey.
Here's my take at fixing everything slightly related to thread handling
up:
- full switch to kthread infrastructure
- remove unused semaphore as mutex and waitqueue in long_delay -
in fact that whole function should just go away as the user would
be a lot more happy with just msleep_interruptible.
- use wake_up_process for waking the thread
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/pci/hotplug/cpqphp_ctrl.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/cpqphp_ctrl.c 2007-04-22 12:46:33.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/cpqphp_ctrl.c 2007-04-22 12:53:58.000000000 +0200
@@ -37,6 +37,7 @@
#include <linux/smp_lock.h>
#include <linux/pci.h>
#include <linux/pci_hotplug.h>
+#include <linux/kthread.h>
#include "cpqphp.h"
static u32 configure_new_device(struct controller* ctrl, struct pci_func *func,
@@ -45,34 +46,20 @@ static int configure_new_function(struct
u8 behind_bridge, struct resource_lists *resources);
static void interrupt_event_handler(struct controller *ctrl);
-static struct semaphore event_semaphore; /* mutex for process loop (up if something to process) */
-static struct semaphore event_exit; /* guard ensure thread has exited before calling it quits */
-static int event_finished;
-static unsigned long pushbutton_pending; /* = 0 */
-/* things needed for the long_delay function */
-static struct semaphore delay_sem;
-static wait_queue_head_t delay_wait;
+static struct task_struct *cpqhp_event_thread;
+static unsigned long pushbutton_pending; /* = 0 */
/* delay is in jiffies to wait for */
static void long_delay(int delay)
{
- DECLARE_WAITQUEUE(wait, current);
-
- /* only allow 1 customer into the delay queue at once
- * yes this makes some people wait even longer, but who really cares?
- * this is for _huge_ delays to make the hardware happy as the
- * signals bounce around
+ /*
+ * XXX(hch): if someone is bored please convert all callers
+ * to call msleep_interruptible directly. They really want
+ * to specify timeouts in natural units and spend a lot of
+ * effort converting them to jiffies..
*/
- down (&delay_sem);
-
- init_waitqueue_head(&delay_wait);
-
- add_wait_queue(&delay_wait, &wait);
msleep_interruptible(jiffies_to_msecs(delay));
- remove_wait_queue(&delay_wait, &wait);
-
- up(&delay_sem);
}
@@ -955,8 +942,8 @@ irqreturn_t cpqhp_ctrl_intr(int IRQ, voi
}
if (schedule_flag) {
- up(&event_semaphore);
- dbg("Signal event_semaphore\n");
+ wake_up_process(cpqhp_event_thread);
+ dbg("Waking even thread");
}
return IRQ_HANDLED;
}
@@ -1738,7 +1725,7 @@ static u32 remove_board(struct pci_func
static void pushbutton_helper_thread(unsigned long data)
{
pushbutton_pending = data;
- up(&event_semaphore);
+ wake_up_process(cpqhp_event_thread);
}
@@ -1746,16 +1733,14 @@ static void pushbutton_helper_thread(uns
static int event_thread(void* data)
{
struct controller *ctrl;
- lock_kernel();
- daemonize("phpd_event");
-
- unlock_kernel();
while (1) {
dbg("!!!!event_thread sleeping\n");
- down_interruptible (&event_semaphore);
- dbg("event_thread woken finished = %d\n", event_finished);
- if (event_finished) break;
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+
+ if (kthread_should_stop())
+ break;
/* Do stuff here */
if (pushbutton_pending)
cpqhp_pushbutton_thread(pushbutton_pending);
@@ -1764,38 +1749,24 @@ static int event_thread(void* data)
interrupt_event_handler(ctrl);
}
dbg("event_thread signals exit\n");
- up(&event_exit);
return 0;
}
-
int cpqhp_event_start_thread(void)
{
- int pid;
-
- /* initialize our semaphores */
- init_MUTEX(&delay_sem);
- init_MUTEX_LOCKED(&event_semaphore);
- init_MUTEX_LOCKED(&event_exit);
- event_finished=0;
-
- pid = kernel_thread(event_thread, NULL, 0);
- if (pid < 0) {
+ cpqhp_event_thread = kthread_run(event_thread, NULL, "phpd_event");
+ if (IS_ERR(cpqhp_event_thread)) {
err ("Can't start up our event thread\n");
- return -1;
+ return PTR_ERR(cpqhp_event_thread);
}
- dbg("Our event thread pid = %d\n", pid);
+
return 0;
}
void cpqhp_event_stop_thread(void)
{
- event_finished = 1;
- dbg("event_thread finish command given\n");
- up(&event_semaphore);
- dbg("wait for event_thread to exit\n");
- down(&event_exit);
+ kthread_stop(cpqhp_event_thread);
}
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: Remaining straight forward kthread API conversions... [message #18466 is a reply to message #18280] |
Sun, 22 April 2007 12:15 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
Looks like you were missing at least the pcie hotplug driver. Another
one of the horrible thread abuses in drivers/pci/hotpug.
- full conversion to kthread infrastructure
- use wake_up_process to wake the thread up
Like most pci hotplug drivers it still uses very race non-atomic variable
assignment to communicated with the thread, but that's something the
maintainers should look into.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_ctrl.c 2007-04-22 11:36:58.000000000 +0200
+++ linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c 2007-04-22 11:42:56.000000000 +0200
@@ -32,14 +32,13 @@
#include <linux/types.h>
#include <linux/smp_lock.h>
#include <linux/pci.h>
+#include <linux/kthread.h>
#include "../pci.h"
#include "pciehp.h"
static void interrupt_event_handler(struct controller *ctrl);
-static struct semaphore event_semaphore; /* mutex for process loop (up if something to process) */
-static struct semaphore event_exit; /* guard ensure thread has exited before calling it quits */
-static int event_finished;
+static struct task_struct *pciehpd_event_thread;
static unsigned long pushbutton_pending; /* = 0 */
static unsigned long surprise_rm_pending; /* = 0 */
@@ -93,8 +92,9 @@ u8 pciehp_handle_attention_button(u8 hp_
info("Button ignore on Slot(%s)\n", slot_name(p_slot));
}
+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);
return 0;
@@ -135,8 +135,9 @@ u8 pciehp_handle_switch_change(u8 hp_slo
taskInfo->event_type = INT_SWITCH_CLOSE;
}
+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);
return rc;
}
@@ -178,8 +179,9 @@ u8 pciehp_handle_presence_change(u8 hp_s
taskInfo->event_type = INT_PRESENCE_OFF;
}
+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);
return rc;
}
@@ -217,8 +219,10 @@ u8 pciehp_handle_power_fault(u8 hp_slot,
taskInfo->event_type = INT_POWER_FAULT;
info("power fault bit %x set\n", hp_slot);
}
+
+ /* signal event thread that new event is posted */
if (rc)
- up(&event_semaphore); /* signal event thread that new event is posted */
+ wake_up_process(pciehpd_event_thread);
return rc;
}
@@ -362,7 +366,7 @@ static void pushbutton_helper_thread(uns
{
pushbutton_pending = data;
- up(&event_semaphore);
+ wake_up_process(pciehpd_event_thread);
}
/**
@@ -452,19 +456,14 @@ static void pciehp_surprise_rm_thread(un
/* this is the main worker thread */
-static int event_thread(void* data)
+static int event_thread(void *data)
{
struct controller *ctrl;
- lock_kernel();
- daemonize("pciehpd_event");
-
- unlock_kernel();
while (1) {
- dbg("!!!!event_thread sleeping\n");
- down_interruptible (&event_semaphore);
- dbg("event_thread woken finished = %d\n", event_finished);
- if (event_finished || signal_pending(current))
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ if (kthread_should_stop())
break;
/* Do stuff here */
if (pushbutton_pending)
@@ -476,24 +475,15 @@ static int event_thread(void* data)
interrupt_event_handler(ctrl);
}
dbg("event_thread signals exit\n");
- up(&event_exit);
return 0;
}
int pciehp_event_start_thread(void)
{
- int pid;
-
- /* initialize our semaphores */
- init_MUTEX_LOCKED(&event_exit);
- event_finished=0;
-
- init_MUTEX_LOCKED(&event_semaphore);
- pid = kernel_thread(event_thread, NULL, 0);
-
- if (pid < 0) {
+ pciehpd_event_thread = kthread_run(event_thread, NULL, "pciehpd_event");
+ if (IS_ERR(pciehpd_event_thread)) {
err ("Can't start up our event thread\n");
- return -1;
+ return PTR_ERR(pciehpd_event_thread);
}
return 0;
}
@@ -501,9 +491,7 @@ int pciehp_event_start_thread(void)
void pciehp_event_stop_thread(void)
{
- event_finished = 1;
- up(&event_semaphore);
- down(&event_exit);
+ kthread_stop(pciehpd_event_thread);
}
@@ -624,7 +612,7 @@ static void interrupt_event_handler(stru
dbg("Surprise Removal\n");
if (p_slot) {
surprise_rm_pending = (unsigned long) p_slot;
- up(&event_semaphore);
+ wake_up_process(pciehpd_event_thread);
update_slot_info(p_slot);
}
}
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] mtd_blkdevs: Convert to use the kthread API [message #18467 is a reply to message #18283] |
Sun, 22 April 2007 12:24 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 12:55:28AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
>
> thread_run is used intead of kernel_thread, daemonize, and mucking
> around blocking signals directly.
This is the full conversion I sent to Dave in April 2006, but never got
any feedback to:
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/mtd/mtd_blkdevs.c
===================================================================
--- linux-2.6.orig/drivers/mtd/mtd_blkdevs.c 2007-01-29 10:03:52.000000000 +0100
+++ linux-2.6/drivers/mtd/mtd_blkdevs.c 2007-04-22 13:22:03.000000000 +0200
@@ -20,6 +20,7 @@
#include <linux/hdreg.h>
#include <linux/init.h>
#include <linux/mutex.h>
+#include <linux/kthread.h>
#include <asm/uaccess.h>
static LIST_HEAD(blktrans_majors);
@@ -28,9 +29,7 @@ extern struct mutex mtd_table_mutex;
extern struct mtd_info *mtd_table[];
struct mtd_blkcore_priv {
- struct completion thread_dead;
- int exiting;
- wait_queue_head_t thread_wq;
+ struct task_struct *thread;
struct request_queue *rq;
spinlock_t queue_lock;
};
@@ -83,38 +82,19 @@ static int mtd_blktrans_thread(void *arg
/* we might get involved when memory gets low, so use PF_MEMALLOC */
current->flags |= PF_MEMALLOC | PF_NOFREEZE;
- daemonize("%sd", tr->name);
-
- /* daemonize() doesn't do this for us since some kernel threads
- actually want to deal with signals. We can't just call
- exit_sighand() since that'll cause an oops when we finally
- do exit. */
- spin_lock_irq(¤t->sighand->siglock);
- sigfillset(¤t->blocked);
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
-
spin_lock_irq(rq->queue_lock);
-
- while (!tr->blkcore_priv->exiting) {
+ while (!kthread_should_stop()) {
struct request *req;
struct mtd_blktrans_dev *dev;
int res = 0;
- DECLARE_WAITQUEUE(wait, current);
req = elv_next_request(rq);
if (!req) {
- add_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
-
spin_unlock_irq(rq->queue_lock);
-
+ set_current_state(TASK_INTERRUPTIBLE);
schedule();
- remove_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
-
spin_lock_irq(rq->queue_lock);
-
continue;
}
@@ -133,13 +113,13 @@ static int mtd_blktrans_thread(void *arg
}
spin_unlock_irq(rq->queue_lock);
- complete_and_exit(&tr->blkcore_priv->thread_dead, 0);
+ return 0;
}
static void mtd_blktrans_request(struct request_queue *rq)
{
struct mtd_blktrans_ops *tr = rq->queuedata;
- wake_up(&tr->blkcore_priv->thread_wq);
+ wake_up_process(tr->blkcore_priv->thread);
}
@@ -388,8 +368,6 @@ int register_mtd_blktrans(struct mtd_blk
return ret;
}
spin_lock_init(&tr->blkcore_priv->queue_lock);
- init_completion(&tr->blkcore_priv->thread_dead);
- init_waitqueue_head(&tr->blkcore_priv->thread_wq);
tr->blkcore_priv->rq = blk_init_queue(mtd_blktrans_request, &tr->blkcore_priv->queue_lock);
if (!tr->blkcore_priv->rq) {
@@ -403,13 +381,14 @@ int register_mtd_blktrans(struct mtd_blk
blk_queue_hardsect_size(tr->blkcore_priv->rq, tr->blksize);
tr->blkshift = ffs(tr->blksize) - 1;
- ret = kernel_thread(mtd_blktrans_thread, tr, CLONE_KERNEL);
- if (ret < 0) {
+ tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,
+ "%sd", tr->name);
+ if (IS_ERR(tr->blkcore_priv->thread)) {
blk_cleanup_queue(tr->blkcore_priv->rq);
unregister_blkdev(tr->major, tr->name);
kfree(tr->blkcore_priv);
mutex_unlock(&mtd_table_mutex);
- return ret;
+ return PTR_ERR(tr->blkcore_priv->thread);
}
INIT_LIST_HEAD(&tr->devs);
@@ -432,9 +411,7 @@ int deregister_mtd_blktrans(struct mtd_b
mutex_lock(&mtd_table_mutex);
/* Clean up the kernel thread */
- tr->blkcore_priv->exiting = 1;
- wake_up(&tr->blkcore_priv->thread_wq);
- wait_for_completion(&tr->blkcore_priv->thread_dead);
+ kthread_stop(tr->blkcore_priv->thread);
/* Remove it from the list of active majors */
list_del(&tr->list);
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
|
Re: [PATCH] mtd_blkdevs: Convert to use the kthread API [message #18470 is a reply to message #18467] |
Sun, 22 April 2007 13:23 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
David Woodhouse
Messages: 1 Registered: April 2007
|
Junior Member |
|
|
On Sun, 2007-04-22 at 13:24 +0100, Christoph Hellwig wrote:
> This is the full conversion I sent to Dave in April 2006, but never
> got any feedback to:
Sorry about that; I need prodding sometimes. I'll provide some now...
Can you show me why the thread won't now miss a wakeup if it goes to
sleep just as a new request is added to its queue?
Having already applied Eric's patch, this is the delta to yours...
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 1aa018a..d065dba 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -29,9 +29,7 @@ extern struct mutex mtd_table_mutex;
extern struct mtd_info *mtd_table[];
struct mtd_blkcore_priv {
- struct completion thread_dead;
- int exiting;
- wait_queue_head_t thread_wq;
+ struct task_struct *thread;
struct request_queue *rq;
spinlock_t queue_lock;
};
@@ -85,26 +83,18 @@ static int mtd_blktrans_thread(void *arg)
current->flags |= PF_MEMALLOC | PF_NOFREEZE;
spin_lock_irq(rq->queue_lock);
-
- while (!tr->blkcore_priv->exiting) {
+ while (!kthread_should_stop()) {
struct request *req;
struct mtd_blktrans_dev *dev;
int res = 0;
- DECLARE_WAITQUEUE(wait, current);
req = elv_next_request(rq);
if (!req) {
- add_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
-
spin_unlock_irq(rq->queue_lock);
-
+ set_current_state(TASK_INTERRUPTIBLE);
schedule();
- remove_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
-
spin_lock_irq(rq->queue_lock);
-
continue;
}
@@ -123,13 +113,13 @@ static int mtd_blktrans_thread(void *arg)
}
spin_unlock_irq(rq->queue_lock);
- complete_and_exit(&tr->blkcore_priv->thread_dead, 0);
+ return 0;
}
static void mtd_blktrans_request(struct request_queue *rq)
{
struct mtd_blktrans_ops *tr = rq->queuedata;
- wake_up(&tr->blkcore_priv->thread_wq);
+ wake_up_process(tr->blkcore_priv->thread);
}
@@ -355,7 +345,6 @@ static struct mtd_notifier blktrans_notifier = {
int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
{
- struct task_struct *task;
int ret, i;
/* Register the notifier if/when the first device type is
@@ -379,8 +368,6 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
return ret;
}
spin_lock_init(&tr->blkcore_priv->queue_lock);
- init_completion(&tr->blkcore_priv->thread_dead);
- init_waitqueue_head(&tr->blkcore_priv->thread_wq);
tr->blkcore_priv->rq = blk_init_queue(mtd_blktrans_request, &tr->blkcore_priv->queue_lock);
if (!tr->blkcore_priv->rq) {
@@ -394,13 +381,14 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
blk_queue_hardsect_size(tr->blkcore_priv->rq, tr->blksize);
tr->blkshift = ffs(tr->blksize) - 1;
- task = kthread_run(mtd_blktrans_thread, tr, "%sd", tr->name);
- if (IS_ERR(task)) {
+ tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,
+ "%sd", tr->name);
+ if (IS_ERR(tr->blkcore_priv->thread)) {
blk_cleanup_queue(tr->blkcore_priv->rq);
unregister_blkdev(tr->major, tr->name);
kfree(tr->blkcore_priv);
mutex_unlock(&mtd_table_mutex);
- return PTR_ERR(task);
+ return PTR_ERR(tr->blkcore_priv->thread);
}
INIT_LIST_HEAD(&tr->devs);
@@ -423,9 +411,7 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
mutex_lock(&mtd_table_mutex);
/* Clean up the kernel thread */
- tr->blkcore_priv->exiting = 1;
- wake_up(&tr->blkcore_priv->thread_wq);
- wait_for_completion(&tr->blkcore_priv->thread_dead);
+ kthread_stop(tr->blkcore_priv->thread);
/* Remove it from the list of active majors */
list_del(&tr->list);
--
dwmw2
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
|
|
|
Re: [PATCH] i386 voyager: Convert the monitor thread to use the kthread API [message #18476 is a reply to message #18282] |
Sun, 22 April 2007 19:30 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 12:55:27AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
>
> This patch just trivially replaces kernel_thread and daemonize
> with a single call to kthread_run.
Here's a better patch that does the full kthread conversion +
switch to wake_up_process. Only compile tested of course due to
lack of voyager hardware.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/arch/i386/mach-voyager/voyager_cat.c
===================================================================
--- linux-2.6.orig/arch/i386/mach-voyager/voyager_cat.c 2007-04-22 15:19:28.000000000 +0200
+++ linux-2.6/arch/i386/mach-voyager/voyager_cat.c 2007-04-22 15:27:03.000000000 +0200
@@ -1111,7 +1111,7 @@ voyager_cat_do_common_interrupt(void)
printk(KERN_ERR "Voyager front panel switch turned off\n");
voyager_status.switch_off = 1;
voyager_status.request_from_kernel = 1;
- up(&kvoyagerd_sem);
+ wake_up_process(voyager_thread);
}
/* Tell the hardware we're taking care of the
* shutdown, otherwise it will power the box off
@@ -1157,7 +1157,7 @@ voyager_cat_do_common_interrupt(void)
outb(VOYAGER_CAT_END, CAT_CMD);
voyager_status.power_fail = 1;
voyager_status.request_from_kernel = 1;
- up(&kvoyagerd_sem);
+ wake_up_process(voyager_thread);
}
Index: linux-2.6/arch/i386/mach-voyager/voyager_thread.c
===================================================================
--- linux-2.6.orig/arch/i386/mach-voyager/voyager_thread.c 2007-04-22 15:15:24.000000000 +0200
+++ linux-2.6/arch/i386/mach-voyager/voyager_thread.c 2007-04-22 15:25:51.000000000 +0200
@@ -24,33 +24,16 @@
#include <linux/kmod.h>
#include <linux/completion.h>
#include <linux/sched.h>
+#include <linux/kthread.h>
#include <asm/desc.h>
#include <asm/voyager.h>
#include <asm/vic.h>
#include <asm/mtrr.h>
#include <asm/msr.h>
-#define THREAD_NAME "kvoyagerd"
-/* external variables */
-int kvoyagerd_running = 0;
-DECLARE_MUTEX_LOCKED(kvoyagerd_sem);
-
-static int thread(void *);
-
-static __u8 set_timeout = 0;
-
-/* Start the machine monitor thread. Return 1 if OK, 0 if fail */
-static int __init
-voyager_thread_start(void)
-{
- if(kernel_thread(thread, NULL, CLONE_KERNEL) < 0) {
- /* This is serious, but not fatal */
- printk(KERN_ERR "Voyager: Failed to create system monitor thread!!!\n");
- return 1;
- }
- return 0;
-}
+struct task_struct *voyager_thread;
+static __u8 set_timeout;
static int
execute(const char *string)
@@ -110,31 +93,15 @@ check_continuing_condition(void)
}
}
-static void
-wakeup(unsigned long unused)
-{
- up(&kvoyagerd_sem);
-}
-
static int
thread(void *unused)
{
- struct timer_list wakeup_timer;
-
- kvoyagerd_running = 1;
-
- daemonize(THREAD_NAME);
-
- set_timeout = 0;
-
- init_timer(&wakeup_timer);
-
- sigfillset(¤t->blocked);
-
printk(KERN_NOTICE "Voyager starting monitor thread\n");
- for(;;) {
- down_interruptible(&kvoyagerd_sem);
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(set_timeout ? HZ : MAX_SCHEDULE_TIMEOUT);
+
VDEBUG(("Voyager Daemon awoken\n"));
if(voyager_status.request_from_kernel == 0) {
/* probably awoken from timeout */
@@ -143,20 +110,26 @@ thread(void *unused)
check_from_kernel();
voyager_status.request_from_kernel = 0;
}
- if(set_timeout) {
- del_timer(&wakeup_timer);
- wakeup_timer.expires = HZ + jiffies;
- wakeup_timer.function = wakeup;
- add_timer(&wakeup_timer);
- }
}
}
+static int __init
+voyager_thread_start(void)
+{
+ voyager_thread = kthread_run(thread, NULL, "kvoyagerd");
+ if (IS_ERR(voyager_thread)) {
+ printk(KERN_ERR "Voyager: Failed to create system monitor thread.\n");
+ return PTR_ERR(voyager_thread);
+ }
+ return 0;
+}
+
+
static void __exit
voyager_thread_stop(void)
{
- /* FIXME: do nothing at the moment */
+ kthread_stop(voyager_thread);
}
module_init(voyager_thread_start);
-//module_exit(voyager_thread_stop);
+module_exit(voyager_thread_stop);
Index: linux-2.6/include/asm-i386/voyager.h
===================================================================
--- linux-2.6.orig/include/asm-i386/voyager.h 2007-04-22 15:18:39.000000000 +0200
+++ linux-2.6/include/asm-i386/voyager.h 2007-04-22 15:24:13.000000000 +0200
@@ -487,15 +487,11 @@ extern struct voyager_qic_cpi *voyager_q
extern struct voyager_SUS *voyager_SUS;
/* variables exported always */
+extern struct task_struct *voyager_thread;
extern int voyager_level;
-extern int kvoyagerd_running;
-extern struct semaphore kvoyagerd_sem;
extern struct voyager_status voyager_status;
-
-
/* functions exported by the voyager and voyager_smp modules */
-
extern int voyager_cat_readb(__u8 module, __u8 asic, int reg);
extern void voyager_cat_init(void);
extern void voyager_detect(struct voyager_bios_info *);
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] sas_scsi_host: Convert to use the kthread API [message #18477 is a reply to message #18388] |
Sun, 22 April 2007 19:38 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 05:37:53PM -0700, Andrew Morton wrote:
> On Thu, 19 Apr 2007 01:58:38 -0600
> "Eric W. Biederman" <ebiederm@xmission.com> wrote:
>
> > From: Eric W. Biederman <ebiederm@xmission.com>
> >
> > This patch modifies the sas scsi host thread startup
> > to use kthread_run not kernel_thread and deamonize.
> > kthread_run is slightly simpler and more maintainable.
> >
>
> Again, I'll rename this to "partially convert...". This driver should be
> using kthread_should_stop() and kthread_stop() rather than the
> apparently-unnecessary ->queue_thread_kill thing.
>
> This driver was merged two and a half years after the kthread API was
> available. Our coding-vs-reviewing effort is out of balance.
Here's a full conversion.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/scsi/libsas/sas_scsi_host.c
===================================================================
--- linux-2.6.orig/drivers/scsi/libsas/sas_scsi_host.c 2007-04-22 20:30:39.000000000 +0200
+++ linux-2.6/drivers/scsi/libsas/sas_scsi_host.c 2007-04-22 20:36:51.000000000 +0200
@@ -23,6 +23,8 @@
*
*/
+#include <linux/kthread.h>
+
#include "sas_internal.h"
#include <scsi/scsi_host.h>
@@ -184,7 +186,7 @@ static int sas_queue_up(struct sas_task
list_add_tail(&task->list, &core->task_queue);
core->task_queue_size += 1;
spin_unlock_irqrestore(&core->task_queue_lock, flags);
- up(&core->queue_thread_sema);
+ wake_up_process(core->queue_thread);
return 0;
}
@@ -819,7 +821,7 @@ static void sas_queue(struct sas_ha_stru
struct sas_internal *i = to_sas_internal(core->shost->transportt);
spin_lock_irqsave(&core->task_queue_lock, flags);
- while (!core->queue_thread_kill &&
+ while (!kthread_should_stop() &&
!list_empty(&core->task_queue)) {
can_queue = sas_ha->lldd_queue_size - core->task_queue_size;
@@ -858,8 +860,6 @@ static void sas_queue(struct sas_ha_stru
spin_unlock_irqrestore(&core->task_queue_lock, flags);
}
-static DECLARE_COMPLETION(queue_th_comp);
-
/**
* sas_queue_thread -- The Task Collector thread
* @_sas_ha: pointer to struct sas_ha
@@ -867,40 +867,33 @@ static DECLARE_COMPLETION(queue_th_comp)
static int sas_queue_thread(void *_sas_ha)
{
struct sas_ha_struct *sas_ha = _sas_ha;
- struct scsi_core *core = &sas_ha->core;
- daemonize("sas_queue_%d", core->shost->host_no);
current->flags |= PF_NOFREEZE;
- complete(&queue_th_comp);
-
while (1) {
- down_interruptible(&core->queue_thread_sema);
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
sas_queue(sas_ha);
- if (core->queue_thread_kill)
+ if (kthread_should_stop())
break;
}
- complete(&queue_th_comp);
-
return 0;
}
int sas_init_queue(struct sas_ha_struct *sas_ha)
{
- int res;
struct scsi_core *core = &sas_ha->core;
spin_lock_init(&core->task_queue_lock);
core->task_queue_size = 0;
INIT_LIST_HEAD(&core->task_queue);
- init_MUTEX_LOCKED(&core->queue_thread_sema);
- res = kernel_thread(sas_queue_thread, sas_ha, 0);
- if (res >= 0)
- wait_for_completion(&queue_th_comp);
-
- return res < 0 ? res : 0;
+ core->queue_thread = kthread_run(sas_queue_thread, sas_ha,
+ "sas_queue_%d", core->shost->host_no);
+ if (IS_ERR(core->queue_thread))
+ return PTR_ERR(core->queue_thread);
+ return 0;
}
void sas_shutdown_queue(struct sas_ha_struct *sas_ha)
@@ -909,10 +902,7 @@ void sas_shutdown_queue(struct sas_ha_st
struct scsi_core *core = &sas_ha->core;
struct sas_task *task, *n;
- init_completion(&queue_th_comp);
- core->queue_thread_kill = 1;
- up(&core->queue_thread_sema);
- wait_for_completion(&queue_th_comp);
+ kthread_stop(core->queue_thread);
if (!list_empty(&core->task_queue))
SAS_DPRINTK("HA: %llx: scsi core task queue is NOT empty!?\n",
Index: linux-2.6/include/scsi/libsas.h
===================================================================
--- linux-2.6.orig/include/scsi/libsas.h 2007-04-22 20:32:41.000000000 +0200
+++ linux-2.6/include/scsi/libsas.h 2007-04-22 20:32:59.000000000 +0200
@@ -314,8 +314,7 @@ struct scsi_core {
struct list_head task_queue;
int task_queue_size;
- struct semaphore queue_thread_sema;
- int queue_thread_kill;
+ struct task_struct *queue_thread;
};
struct sas_ha_event {
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] mtd_blkdevs: Convert to use the kthread API [message #18478 is a reply to message #18467] |
Sun, 22 April 2007 19:40 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Sun, Apr 22, 2007 at 01:24:53PM +0100, Christoph Hellwig wrote:
> On Thu, Apr 19, 2007 at 12:55:28AM -0600, Eric W. Biederman wrote:
> > From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> >
> > thread_run is used intead of kernel_thread, daemonize, and mucking
> > around blocking signals directly.
>
> This is the full conversion I sent to Dave in April 2006, but never got
> any feedback to:
Here's a slightly updated version that corrects the set_current_state
placement as discussed with Dave on irc:
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/mtd/mtd_blkdevs.c
===================================================================
--- linux-2.6.orig/drivers/mtd/mtd_blkdevs.c 2007-01-29 10:03:52.000000000 +0100
+++ linux-2.6/drivers/mtd/mtd_blkdevs.c 2007-04-22 20:39:20.000000000 +0200
@@ -20,6 +20,7 @@
#include <linux/hdreg.h>
#include <linux/init.h>
#include <linux/mutex.h>
+#include <linux/kthread.h>
#include <asm/uaccess.h>
static LIST_HEAD(blktrans_majors);
@@ -28,9 +29,7 @@ extern struct mutex mtd_table_mutex;
extern struct mtd_info *mtd_table[];
struct mtd_blkcore_priv {
- struct completion thread_dead;
- int exiting;
- wait_queue_head_t thread_wq;
+ struct task_struct *thread;
struct request_queue *rq;
spinlock_t queue_lock;
};
@@ -83,38 +82,19 @@ static int mtd_blktrans_thread(void *arg
/* we might get involved when memory gets low, so use PF_MEMALLOC */
current->flags |= PF_MEMALLOC | PF_NOFREEZE;
- daemonize("%sd", tr->name);
-
- /* daemonize() doesn't do this for us since some kernel threads
- actually want to deal with signals. We can't just call
- exit_sighand() since that'll cause an oops when we finally
- do exit. */
- spin_lock_irq(¤t->sighand->siglock);
- sigfillset(¤t->blocked);
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
-
spin_lock_irq(rq->queue_lock);
-
- while (!tr->blkcore_priv->exiting) {
+ while (!kthread_should_stop()) {
struct request *req;
struct mtd_blktrans_dev *dev;
int res = 0;
- DECLARE_WAITQUEUE(wait, current);
req = elv_next_request(rq);
if (!req) {
- add_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
set_current_state(TASK_INTERRUPTIBLE);
-
spin_unlock_irq(rq->queue_lock);
-
schedule();
- remove_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
-
spin_lock_irq(rq->queue_lock);
-
continue;
}
@@ -133,13 +113,13 @@ static int mtd_blktrans_thread(void *arg
}
spin_unlock_irq(rq->queue_lock);
- complete_and_exit(&tr->blkcore_priv->thread_dead, 0);
+ return 0;
}
static void mtd_blktrans_request(struct request_queue *rq)
{
struct mtd_blktrans_ops *tr = rq->queuedata;
- wake_up(&tr->blkcore_priv->thread_wq);
+ wake_up_process(tr->blkcore_priv->thread);
}
@@ -388,8 +368,6 @@ int register_mtd_blktrans(struct mtd_blk
return ret;
}
spin_lock_init(&tr->blkcore_priv->queue_lock);
- init_completion(&tr->blkcore_priv->thread_dead);
- init_waitqueue_head(&tr->blkcore_priv->thread_wq);
tr->blkcore_priv->rq = blk_init_queue(mtd_blktrans_request, &tr->blkcore_priv->queue_lock);
if (!tr->blkcore_priv->rq) {
@@ -403,13 +381,14 @@ int register_mtd_blktrans(struct mtd_blk
blk_queue_hardsect_size(tr->blkcore_priv->rq, tr->blksize);
tr->blkshift = ffs(tr->blksize) - 1;
- ret = kernel_thread(mtd_blktrans_thread, tr, CLONE_KERNEL);
- if (ret < 0) {
+ tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,
+ "%sd", tr->name);
+ if (IS_ERR(tr->blkcore_priv->thread)) {
blk_cleanup_queue(tr->blkcore_priv->rq);
unregister_blkdev(tr->major, tr->name);
kfree(tr->blkcore_priv);
mutex_unlock(&mtd_table_mutex);
- return ret;
+ return PTR_ERR(tr->blkcore_priv->thread);
}
INIT_LIST_HEAD(&tr->devs);
@@ -432,9 +411,7 @@ int deregister_mtd_blktrans(struct mtd_b
mutex_lock(&mtd_table_mutex);
/* Clean up the kernel thread */
- tr->blkcore_priv->exiting = 1;
- wake_up(&tr->blkcore_priv->thread_wq);
- wait_for_completion(&tr->blkcore_priv->thread_dead);
+ kthread_stop(tr->blkcore_priv->thread);
/* Remove it from the list of active majors */
list_del(&tr->list);
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
|
[PATCH] kthread: Spontaneous exit support [message #18481 is a reply to message #18479] |
Mon, 23 April 2007 03:12 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
ebiederm
Messages: 1354 Registered: February 2006
|
Senior Member |
|
|
This patch implements the kthread helper functions kthread_start
and kthread_end which make it simple to support a kernel thread
that may decided to exit on it's own before we request it to.
It is still assumed that eventually we will get around to requesting
that the kernel thread stop.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
include/linux/kthread.h | 23 +++++++++++++++++++++++
kernel/kthread.c | 18 ++++++++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index a8ea31d..4f1eff1 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -28,6 +28,29 @@ struct task_struct *kthread_create(int (*threadfn)(void *data),
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
+/**
+ * kthread_start - create and wake a thread.
+ * @threadfn: the function to run until kthread_should_stop().
+ * @data: data ptr for @threadfn.
+ * @namefmt: printf-style name for the thread.
+ *
+ * Description: Convenient wrapper for kthread_create() followed by
+ * get_task_struct() and wake_up_process. kthread_start should be paired
+ * with kthread_end() so we don't leak task structs.
+ *
+ * Returns the kthread or ERR_PTR(-ENOMEM).
+ */
+#define kthread_start(threadfn, data, namefmt, ...) \
+({ \
+ struct task_struct *__k \
+ = kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
+ if (!IS_ERR(__k)) { \
+ get_task_struct(__k); \
+ wake_up_process(__k); \
+ } \
+ __k; \
+})
+int kthread_end(struct task_struct *k);
static inline int __kthread_should_stop(struct task_struct *tsk)
{
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 9b3c19f..d6d63c6 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -179,6 +179,24 @@ int kthread_stop(struct task_struct *tsk)
}
EXPORT_SYMBOL(kthread_stop);
+/**
+ * kthread_end - signal a kthread and wait for it to exit.
+ * @task: The kthread to end.
+ *
+ * Description: Convenient wrapper for kthread_stop() followed by
+ * put_task_struct(). Returns the kthread exit code.
+ *
+ * kthread_start()/kthread_end() can handle kthread that spontaneously exit
+ * before the kthread is requested to terminate.
+ */
+int kthread_end(struct task_struct *task)
+{
+ int ret;
+ ret = kthread_stop(task);
+ put_task_struct(task);
+ return ret;
+}
+EXPORT_SYMBOL(kthread_end);
static __init void kthreadd_setup(void)
{
--
1.5.0.g53756
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] bluetooth rfcomm: Convert to kthread API. [message #18482 is a reply to message #18378] |
Sun, 22 April 2007 20:14 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 04:12:53PM -0700, Andrew Morton wrote:
> On Thu, 19 Apr 2007 01:58:54 -0600
> "Eric W. Biederman" <ebiederm@xmission.com> wrote:
>
> > From: Eric W. Biederman <ebiederm@xmission.com>
> >
> > This patch starts krfcommd using kthread_run instead of a combination
> > of kernel_thread and daemonize making the code slightly simpler
> > and more maintainable.
>
> gargh, the more I look at these things, the more I agree with Christoph.
Hehe. Here's a patch to do the full kthread conversion for rfcomm, it
doesn't have the asynchrnous termination issues the other bluetooth drivers
have. Also handle init failures in rfcomm while we're at it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/net/bluetooth/rfcomm/core.c
===================================================================
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c 2007-04-22 21:01:31.000000000 +0200
+++ linux-2.6/net/bluetooth/rfcomm/core.c 2007-04-22 21:12:30.000000000 +0200
@@ -37,6 +37,7 @@
#include <linux/device.h>
#include <linux/net.h>
#include <linux/mutex.h>
+#include <linux/kthread.h>
#include <net/sock.h>
#include <asm/uaccess.h>
@@ -67,7 +68,6 @@ static DEFINE_MUTEX(rfcomm_mutex);
static unsigned long rfcomm_event;
static LIST_HEAD(session_list);
-static atomic_t terminate, running;
static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len);
static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci);
@@ -1846,26 +1846,6 @@ static inline void rfcomm_process_sessio
rfcomm_unlock();
}
-static void rfcomm_worker(void)
-{
- BT_DBG("");
-
- while (!atomic_read(&terminate)) {
- if (!test_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event)) {
- /* No pending events. Let's sleep.
- * Incoming connections and data will wake us up. */
- set_current_state(TASK_INTERRUPTIBLE);
- schedule();
- }
-
- /* Process stuff */
- clear_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event);
- rfcomm_process_sessions();
- }
- set_current_state(TASK_RUNNING);
- return;
-}
-
static int rfcomm_add_listener(bdaddr_t *ba)
{
struct sockaddr_l2 addr;
@@ -1931,23 +1911,27 @@ static void rfcomm_kill_listener(void)
static int rfcomm_run(void *unused)
{
- rfcomm_thread = current;
-
- atomic_inc(&running);
-
- daemonize("krfcommd");
set_user_nice(current, -10);
current->flags |= PF_NOFREEZE;
BT_DBG("");
rfcomm_add_listener(BDADDR_ANY);
+ while (!kthread_should_stop()) {
+ if (!test_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event)) {
+ /* No pending events. Let's sleep.
+ * Incoming connections and data will wake us up. */
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ }
- rfcomm_worker();
-
+ /* Process stuff */
+ clear_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event);
+ rfcomm_process_sessions();
+ }
+ set_current_state(TASK_RUNNING);
rfcomm_kill_listener();
- atomic_dec(&running);
return 0;
}
@@ -2052,24 +2036,52 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
/* ---- Initialization ---- */
static int __init rfcomm_init(void)
{
+ int err;
+
l2cap_load();
- hci_register_cb(&rfcomm_cb);
+ err = hci_register_cb(&rfcomm_cb);
+ if (err)
+ goto out;
- kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+ rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
+ if (IS_ERR(rfcomm_thread)) {
+ err = PTR_ERR(rfcomm_thread);
+ goto out_unregister_hci;
+ }
- if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
+ err = class_create_file(bt_class, &class_attr_rfcomm_dlc);
+ if (err < 0) {
BT_ERR("Failed to create RFCOMM info file");
+ goto out_kthread_stop;
+ }
- rfcomm_init_sockets();
+ err = rfcomm_init_sockets();
+ if (err)
+ goto out_remove_sysfs_files;
#ifdef CONFIG_BT_RFCOMM_TTY
- rfcomm_init_ttys();
+ err = rfcomm_init_ttys();
+ if (err)
+ goto out_cleanup_sockets;
#endif
BT_INFO("RFCOMM ver %s", VERSION);
return 0;
+
+#ifdef CONFIG_BT_RFCOMM_TTY
+ out_cleanup_sockets:
+ rfcomm_cleanup_sockets();
+#endif
+ out_remove_sysfs_files:
+ class_remove_file(bt_class, &class_attr_rfcomm_dlc);
+ out_unregister_hci:
+ hci_unregister_cb(&rfcomm_cb);
+ out_kthread_stop:
+ kthread_stop(rfcomm_thread);
+ out:
+ return err;
}
static void __exit rfcomm_exit(void)
@@ -2077,15 +2089,7 @@ static void __exit rfcomm_exit(void)
class_remove_file(bt_class, &class_attr_rfcomm_dlc);
hci_unregister_cb(&rfcomm_cb);
-
- /* Terminate working thread.
- * ie. Set terminate flag and wake it up */
- atomic_inc(&terminate);
- rfcomm_schedule(RFCOMM_SCHED_STATE);
-
- /* Wait until thread is running */
- while (atomic_read(&running))
- schedule();
+ kthread_stop(rfcomm_thread);
#ifdef CONFIG_BT_RFCOMM_TTY
rfcomm_cleanup_ttys();
_______________________________________________
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 #18485 is a reply to message #18331] |
Sun, 22 April 2007 20:36 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Thu, Apr 19, 2007 at 01:58:44AM -0600, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com>
>
> This patch starts the xpc kernel threads using kthread_run
> not a combination of kernel_thread and daemonize. Resuling
> in slightly simpler and more maintainable code.
This driver is a really twisted maze. It has a lot of threads,
some of them running through the whole lifetime of the driver,
some short-lived and some in a sort of a pool.
The patch below fixes up the long-lived thread as well as fixing
gazillions of leaks in the init routine by switching to proper
goto-based unwinding.
Note that thread pools are something we have in a few places,
and might be worth handling in the core kthread infrastructure,
as tearing down pools will get a bit complicated using the
kthread APIs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/arch/ia64/sn/kernel/xpc_main.c
===================================================================
--- linux-2.6.orig/arch/ia64/sn/kernel/xpc_main.c 2007-04-22 21:19:22.000000000 +0200
+++ linux-2.6/arch/ia64/sn/kernel/xpc_main.c 2007-04-22 21:33:54.000000000 +0200
@@ -55,6 +55,7 @@
#include <linux/delay.h>
#include <linux/reboot.h>
#include <linux/completion.h>
+#include <linux/kthread.h>
#include <asm/sn/intr.h>
#include <asm/sn/sn_sal.h>
#include <asm/kdebug.h>
@@ -159,16 +160,14 @@ static struct ctl_table_header *xpc_sysc
int xpc_disengage_request_timedout;
/* #of IRQs received */
-static atomic_t xpc_act_IRQ_rcvd;
+static atomic_t xpc_act_IRQ_rcvd = ATOMIC_INIT(0);
/* IRQ handler notifies this wait queue on receipt of an IRQ */
static DECLARE_WAIT_QUEUE_HEAD(xpc_act_IRQ_wq);
+static struct task_struct *xpc_hb_checker_thread;
static unsigned long xpc_hb_check_timeout;
-/* notification that the xpc_hb_checker thread has exited */
-static DECLARE_COMPLETION(xpc_hb_checker_exited);
-
/* notification that the xpc_discovery thread has exited */
static DECLARE_COMPLETION(xpc_discovery_exited);
@@ -250,17 +249,10 @@ xpc_hb_checker(void *ignore)
int new_IRQ_count;
int force_IRQ=0;
-
/* this thread was marked active by xpc_hb_init() */
-
- daemonize(XPC_HB_CHECK_THREAD_NAME);
-
- set_cpus_allowed(current, cpumask_of_cpu(XPC_HB_CHECK_CPU));
-
xpc_hb_check_timeout = jiffies + (xpc_hb_check_interval * HZ);
- while (!(volatile int) xpc_exiting) {
-
+ while (!kthread_should_stop()) {
dev_dbg(xpc_part, "woke up with %d ticks rem; %d IRQs have "
"been received\n",
(int) (xpc_hb_check_timeout - jiffies),
@@ -304,14 +296,10 @@ xpc_hb_checker(void *ignore)
(void) wait_event_interruptible(xpc_act_IRQ_wq,
(last_IRQ_count < atomic_read(&xpc_act_IRQ_rcvd) ||
jiffies >= xpc_hb_check_timeout ||
- (volatile int) xpc_exiting));
+ kthread_should_stop()));
}
dev_dbg(xpc_part, "heartbeat checker is exiting\n");
-
-
- /* mark this thread as having exited */
- complete(&xpc_hb_checker_exited);
return 0;
}
@@ -966,9 +954,7 @@ xpc_do_exit(enum xpc_retval reason)
/* wait for the discovery thread to exit */
wait_for_completion(&xpc_discovery_exited);
- /* wait for the heartbeat checker thread to exit */
- wait_for_completion(&xpc_hb_checker_exited);
-
+ kthread_stop(xpc_hb_checker_thread);
/* sleep for a 1/3 of a second or so */
(void) msleep_interruptible(300);
@@ -1219,29 +1205,29 @@ xpc_system_die(struct notifier_block *nb
int __init
xpc_init(void)
{
- int ret;
+ int ret = -ENODEV;
partid_t partid;
struct xpc_partition *part;
pid_t pid;
size_t buf_size;
+ if (!ia64_platform_is("sn2"))
+ goto out;
- if (!ia64_platform_is("sn2")) {
- return -ENODEV;
- }
-
-
+ ret = -ENOMEM;
buf_size = max(XPC_RP_VARS_SIZE,
XPC_RP_HEADER_SIZE + XP_NASID_MASK_BYTES);
xpc_remote_copy_buffer = xpc_kmalloc_cacheline_aligned(buf_size,
GFP_KERNEL, &xpc_remote_copy_buffer_base);
- if (xpc_remote_copy_buffer == NULL)
- return -ENOMEM;
+ if (!xpc_remote_copy_buffer)
+ goto out;
snprintf(xpc_part->bus_id, BUS_ID_SIZE, "part");
snprintf(xpc_chan->bus_id, BUS_ID_SIZE, "chan");
xpc_sysctl = register_sysctl_table(xpc_sys_dir);
+ if (!xpc_sysctl)
+ goto out_free_remote_buffer;
/*
* The first few fields of each entry of xpc_partitions[] need to
@@ -1278,12 +1264,6 @@ xpc_init(void)
xpc_allow_IPI_ops();
/*
- * Interrupts being processed will increment this atomic variable and
- * awaken the heartbeat thread which will process the interrupts.
- */
- atomic_set(&xpc_act_IRQ_rcvd, 0);
-
- /*
* This is safe to do before the xpc_hb_checker thread has started
* because the handler releases a wait queue. If an interrupt is
* received before the thread is waiting, it will not go to sleep,
@@ -1294,15 +1274,7 @@ xpc_init(void)
if (ret != 0) {
dev_err(xpc_part, "can't register ACTIVATE IRQ handler, "
"errno=%d\n", -ret);
-
- xpc_restrict_IPI_ops();
-
- if (xpc_sysctl) {
- unregister_sysctl_table(xpc_sysctl);
- }
-
- kfree(xpc_remote_copy_buffer_base);
- return -EBUSY;
+ goto out_restrict_IPI_ops;
}
/*
@@ -1313,29 +1285,23 @@ xpc_init(void)
xpc_rsvd_page = xpc_rsvd_page_init();
if (xpc_rsvd_page == NULL) {
dev_err(xpc_part, "could not setup our reserved page\n");
-
- free_irq(SGI_XPC_ACTIVATE, NULL);
- xpc_restrict_IPI_ops();
-
- if (xpc_sysctl) {
- unregister_sysctl_table(xpc_sysctl);
- }
-
- kfree(xpc_remote_copy_buffer_base);
- return -EBUSY;
+ ret = -ENOMEM;
+ goto out_free_irq;
}
/* add ourselves to the reboot_notifier_list */
ret = register_reboot_notifier(&xpc_reboot_notifier);
if (ret != 0) {
- dev_warn(xpc_part, "can't register reboot notifier\n");
+ dev_err(xpc_part, "can't register reboot notifier\n");
+ goto out_free_rsvd_page;
}
/* add ourselves to the die_notifier list (i.e., ia64die_chain) */
ret = register_die_notifier(&xpc_die_notifier);
if (ret != 0) {
- dev_warn(xpc_part, "can't register die notifier\n");
+ dev_err(xpc_part, "can't register die notifier\n");
+ goto out_unregister_reboot_notifier;
}
@@ -1353,31 +1319,16 @@ xpc_init(void)
* The real work-horse behind xpc. This processes incoming
* interrupts and monitors remote heartbeats.
*/
- pid = kernel_thread(xpc_hb_checker, NULL, 0);
- if (pid < 0) {
+ xpc_hb_checker_thread = kthread_create(xpc_hb_checker, NULL,
+ XPC_HB_CHECK_THREAD_NAME);
+ if (IS_ERR(xpc_hb_checker_thread)) {
dev_err(xpc_part, "failed while forking hb check thread\n");
-
- /* indicate to others that our reserved page is uninitialized */
- xpc_rsvd_page->vars_pa = 0;
-
- /* take ourselves off of the reboot_notifier_list */
- (void) unregister_reboot_notifier(&xpc_reboot_notifier);
-
- /* take ourselves off of the die_notifier list */
- (void) unregister_die_notifier(&xpc_die_notifier);
-
- del_timer_sync(&xpc_hb_timer);
- free_irq(SGI_XPC_ACTIVATE, NULL);
- xpc_restrict_IPI_ops();
-
- if (xpc_sysctl) {
- unregister_sysctl_table(xpc_sysctl);
- }
-
- kfree(xpc_remote_copy_buffer_base);
- return -EBUSY;
+ ret = PTR_ERR(xpc_hb_checker_thread);
+ goto out_del_hb_timer;
}
+ kthread_bind(xpc_hb_checker_thread, XPC_HB_CHECK_CPU);
+ wake_up_process(xpc_hb_checker_thread);
/*
* Startup a thread that will attempt to discover other partitions to
@@ -1403,6 +1354,29 @@ xpc_init(void)
xpc_initiate_partid_to_nasids);
return 0;
+
+ if (ret != 0) {
+ dev_err(xpc_part, "can't register reboot notifier\n");
+ goto out_free_rsvd_page;
+ }
+
+ out_del_hb_timer:
+ unregister_die_notifier(&xpc_die_notifier);
+ out_unregister_reboot_notifier:
+ unregister_reboot_notifier(&xpc_reboot_notifier);
+ out_free_rsvd_page:
+ /* indicate to others that our reserved page is uninitialized */
+ xpc_rsvd_page->vars_pa = 0;
+ /* XXX(hch): xpc_rsvd_page gets leaked */
+ out_free_irq:
+ free_irq(SGI_XPC_ACTIVATE, NULL);
+ out_restrict_IPI_ops:
+ xpc_restrict_IPI_ops();
+ unregister_sysctl_table(xpc_sysctl);
+ out_free_remote_buffer:
+ kfree(xpc_remote_copy_buffer_base);
+ out:
+ return -EBUSY;
}
module_init(xpc_init);
_______________________________________________
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 #18490 is a reply to message #18463] |
Mon, 23 April 2007 16:19 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Scott Murray
Messages: 4 Registered: April 2007
|
Junior Member |
|
|
On Sun, 22 Apr 2007, Christoph Hellwig wrote:
> On Thu, Apr 19, 2007 at 12:55:29AM -0600, Eric W. Biederman wrote:
> > From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> >
> > kthread_run replaces the kernel_thread and daemonize calls
> > during thread startup.
> >
> > Calls to signal_pending were also removed as it is currently
> > impossible for the cpci_hotplug thread to receive signals.
>
> This drivers thread are a bit of a miss, although a lot better than
> most other pci hotplug drivers :)
Heh, I guess I'll take that as a compliment. :)
> Below is more complete conversion to the kthread infrastructure +
> wake_up_process to wake the thread. Note that we had to keep
> a thread_finished variable because the existing one had dual use.
[snip]
I'm out of the office today, but I'll give it a spin on a test setup
tomorrow.
Thanks,
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] kthread: Spontaneous exit support [message #18491 is a reply to message #18481] |
Mon, 23 April 2007 11:25 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Sun, Apr 22, 2007 at 09:12:55PM -0600, Eric W. Biederman wrote:
>
> This patch implements the kthread helper functions kthread_start
> and kthread_end which make it simple to support a kernel thread
> that may decided to exit on it's own before we request it to.
> It is still assumed that eventually we will get around to requesting
> that the kernel thread stop.
I don't think having to parallel APIs is a good idea, people will
get utterly confused which one to use. Better always grab a reference
in kthread_create and drop it in kthread_stop. For normal thread
no change in behaviour and only slightly more code in the slowpath.
Of course it will need an audit for half-assed kthread conversion
first to avoid task_struct reference count leaks.
In addition to that kthrad_end implementation look wrong. When
the kthread has exited prematurely no one will call complete
on kthread_stop_info.done before it's been setup. Interestingly
the comment there indicates someone thought about threads exiting
early, but it became defunkt during all the rewrites of the
kthread code.
> +/**
> + * kthread_start - create and wake a thread.
> + * @threadfn: the function to run until kthread_should_stop().
> + * @data: data ptr for @threadfn.
> + * @namefmt: printf-style name for the thread.
> + *
> + * Description: Convenient wrapper for kthread_create() followed by
> + * get_task_struct() and wake_up_process. kthread_start should be paired
> + * with kthread_end() so we don't leak task structs.
> + *
> + * Returns the kthread or ERR_PTR(-ENOMEM).
> + */
> +#define kthread_start(threadfn, data, namefmt, ...) \
> +({ \
> + struct task_struct *__k \
> + = kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
> + if (!IS_ERR(__k)) { \
> + get_task_struct(__k); \
> + wake_up_process(__k); \
> + } \
> + __k; \
> +})
> +int kthread_end(struct task_struct *k);
>
> static inline int __kthread_should_stop(struct task_struct *tsk)
> {
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 9b3c19f..d6d63c6 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -179,6 +179,24 @@ int kthread_stop(struct task_struct *tsk)
> }
> EXPORT_SYMBOL(kthread_stop);
>
> +/**
> + * kthread_end - signal a kthread and wait for it to exit.
> + * @task: The kthread to end.
> + *
> + * Description: Convenient wrapper for kthread_stop() followed by
> + * put_task_struct(). Returns the kthread exit code.
> + *
> + * kthread_start()/kthread_end() can handle kthread that spontaneously exit
> + * before the kthread is requested to terminate.
> + */
> +int kthread_end(struct task_struct *task)
> +{
> + int ret;
> + ret = kthread_stop(task);
> + put_task_struct(task);
> + return ret;
> +}
> +EXPORT_SYMBOL(kthread_end);
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
Re: [PATCH] powerpc pseries eeh: Convert to kthread API [message #18493 is a reply to message #18468] |
Mon, 23 April 2007 20:50 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
linas
Messages: 3 Registered: April 2007
|
Junior Member |
|
|
On Sun, Apr 22, 2007 at 01:31:55PM +0100, Christoph Hellwig wrote:
> On Thu, Apr 19, 2007 at 01:58:45AM -0600, Eric W. Biederman wrote:
> > From: Eric W. Biederman <ebiederm@xmission.com>
> >
> > This patch modifies the startup of eehd to use kthread_run
> > not a combination of kernel_thread and daemonize. Making
> > the code slightly simpler and more maintainable.
>
> This one has the same scheme as the various s390 drivers where a thread
> is spawned using a workqueue on demand. I think we should not blindly
> convert it but think a litte more about it.
>
> The first question is obviously, is this really something we want?
> spawning kernel thread on demand without reaping them properly seems
> quite dangerous.
I'm not quite sure what the intent of this patch really is, being
at most a somewhat passing and casual user of kernel threads.
Some background may be useful: (this in reply to some comments from
Andrew Morton)
EEH events are supposed to be very rare, as they correspond to
hardware failures, typically PCI bus parity errors, but also
things like wild DMA's. The code that generates these will limit
them to no more than 6 per hour per pci device. Any more than that,
and the PCI device is permanently disabled (the sysadmin would
need to do something to recover).
The only reason for using threads here is to get the error recovery
out of an interrupt context (where errors may be detected), and then,
an hour later, decrement a counter (which is how we limit these to
6 per hour). Thread reaping is "trivial", the thread just exits
after an hour.
Since these are events rare, I've no particular concern about
performance or resource consumption. The current code seems
to work just fine. :-)
--linas
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
Re: [PATCH] kthread: Spontaneous exit support [message #18494 is a reply to message #18489] |
Mon, 23 April 2007 18:09 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Christoph Hellwig
Messages: 59 Registered: April 2006
|
Member |
|
|
On Mon, Apr 23, 2007 at 11:45:51AM -0600, Eric W. Biederman wrote:
> Ok. Thinking about it I agree with Christoph that parallel API's can
> be a problem.
>
> However we do still need to support kernel threads where kthread_stop will
> never be called. There appear to be a few legitimate cases where
> someone wants to fire off a thread and have it do some work but don't
> care at all for stopping it before it is done.
There's two cases where it's valid that kthread_stop is not called:
a) the user is always builtin and the thread runs until the kernel halts.
examples: voyager, arm ecard
b) the thread is normally started/stopped, e.g. at module_init/module_exit
but there is some reason why it could terminate earlier.
examples: the various bluetooth threads, nfs-related threads that
can be killed using signals
c) we have some kind of asynchronous helper thread.
examples: various s390 drivers, usbatm, therm_pm72
d) a driver has threadpools were we need to start/stop threads on demand.
examples: nfsd, xpc
case a)
is trivial, we can just ignore the refcounting issue.
case b)
is what refcounting the task struct and proper handling in
kthread_stop will deal with.
case c)
should get a new kthread_create_async api which starts a thread
without blocking, so we can get rid of the workqueues in the
s390 drivers. it should probably also be safe to be called from
irq context. What makes this a bit complicated is the need to
make sure no more thread is running in case the caller terminates
(shutdown of the structure it's associated with or module removal)
case d)
should be deal with with a kthread_pool api
_______________________________________________
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 #18495 is a reply to message #18485] |
Mon, 23 April 2007 17:11 ![Go to previous message Go to previous message](/theme/ovz3/images/up.png) ![Go to next message Go to next message](/theme/ovz3/images/down.png) |
Jes Sorensen
Messages: 5 Registered: February 2006
|
Junior Member |
|
|
Christoph Hellwig wrote:
> On Thu, Apr 19, 2007 at 01:58:44AM -0600, Eric W. Biederman wrote:
>> From: Eric W. Biederman <ebiederm@xmission.com>
>>
>> This patch starts the xpc kernel threads using kthread_run
>> not a combination of kernel_thread and daemonize. Resuling
>> in slightly simpler and more maintainable code.
>
> This driver is a really twisted maze. It has a lot of threads,
> some of them running through the whole lifetime of the driver,
> some short-lived and some in a sort of a pool.
>
> The patch below fixes up the long-lived thread as well as fixing
> gazillions of leaks in the init routine by switching to proper
> goto-based unwinding.
>
> Note that thread pools are something we have in a few places,
> and might be worth handling in the core kthread infrastructure,
> as tearing down pools will get a bit complicated using the
> kthread APIs.
Like with the previous patch from Eric, I'm CC'ing the correct people
for this patch (forwarded it in a seperate email). CC'ing irrelevant
lists such as containers@ and not linux-ia64@ makes it somewhat
difficult to get proper reviews of these things.
Russ/Dean/Robin - could one of you provide some feedback to this one
please.
Thanks,
Jes
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
|
|
|
|
|
|
|
Goto Forum:
Current Time: Thu Aug 15 07:29:39 GMT 2024
Total time taken to generate the page: 0.03045 seconds
|