All of lore.kernel.org
 help / color / mirror / Atom feed
* sparc env monitorin vs tasklist_lock
@ 2004-06-15 17:29 Christoph Hellwig
  2004-06-15 17:42 ` Ben Collins
  2004-06-15 18:04 ` Christoph Hellwig
  0 siblings, 2 replies; 3+ messages in thread
From: Christoph Hellwig @ 2004-06-15 17:29 UTC (permalink / raw)
  To: sparclinux

Strange stuff going on here, bbc_envctrl.c and envctrl.c iterate over
the complete task list to check whether the thread has been killed,
yikes!

I've attached a patch to convert it to the kthread infrastructure, but
it has been completely untested, not even compiled.

p.s. the drivers also have other strangenesses like calling execve
directly instead of call_usermodehelper and the bbc driver having it's
own i2c layer..


--- 1.7/drivers/sbus/char/bbc_envctrl.c	2003-09-30 02:35:22 +02:00
+++ edited/drivers/sbus/char/bbc_envctrl.c	2004-06-14 13:33:26 +02:00
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/kthread.h>
 #include <asm/oplib.h>
 #include <asm/ebus.h>
 #define __KERNEL_SYSCALLS__
@@ -457,32 +458,27 @@
 
 static int kenvctrld(void *__unused)
 {
-	daemonize("kenvctrld");
-	allow_signal(SIGKILL);
-	kenvctrld_task = current;
-
 	printk(KERN_INFO "bbc_envctrl: kenvctrld starting...\n");
 	last_warning_jiffies = jiffies - WARN_INTERVAL;
-	for (;;) {
+
+	set_current_state(TASK_INTERRUPTIBLE);
+	while (!kthread_should_stop()) {
 		struct bbc_cpu_temperature *tp;
 		struct bbc_fan_control *fp;
 
-		current->state = TASK_INTERRUPTIBLE;
-		schedule_timeout(POLL_INTERVAL);
-		if (signal_pending(current))
-			break;
-
 		for (tp = all_bbc_temps; tp; tp = tp->next) {
 			get_current_temps(tp);
 			analyze_temps(tp, &last_warning_jiffies);
 		}
 		for (fp = all_bbc_fans; fp; fp = fp->next)
 			maybe_new_fan_speeds(fp);
+
+		set_current_state(TASK_INTERRUPTIBLE);
 	}
+	__set_current_state(TASK_RUNNING);
 	printk(KERN_INFO "bbc_envctrl: kenvctrld exiting...\n");
 
 	fans_full_blast();
-
 	return 0;
 }
 
@@ -576,7 +574,6 @@
 	int temp_index = 0;
 	int fan_index = 0;
 	int devidx = 0;
-	int err = 0;
 
 	while ((echild = bbc_i2c_getdev(devidx++)) != NULL) {
 		if (!strcmp(echild->prom_name, "temperature"))
@@ -584,9 +581,14 @@
 		if (!strcmp(echild->prom_name, "fan-control"))
 			attach_one_fan(echild, fan_index++);
 	}
-	if (temp_index != 0 && fan_index != 0)
-		err = kernel_thread(kenvctrld, NULL, CLONE_FS | CLONE_FILES);
-	return err;
+
+	if (temp_index = 0 || fan_index = 0)
+		return 0;
+
+	kenvctrld_task = kthread_create(kenvctrld, NULL, "%s", "kenvctrld");
+	if (IS_ERR(kenvctrld_task))
+		return PTR_ERR(kenvctrld_task);
+	return 0;
 }
 
 static void destroy_one_temp(struct bbc_cpu_temperature *tp)
@@ -606,28 +608,7 @@
 	struct bbc_cpu_temperature *tp;
 	struct bbc_fan_control *fp;
 
-	if (kenvctrld_task != NULL) {
-		force_sig(SIGKILL, kenvctrld_task);
-		for (;;) {
-			struct task_struct *p;
-			int found = 0;
-
-			read_lock(&tasklist_lock);
-			for_each_process(p) {
-				if (p = kenvctrld_task) {
-					found = 1;
-					break;
-				}
-			}
-			read_unlock(&tasklist_lock);
-			if (!found)
-				break;
-			current->state = TASK_INTERRUPTIBLE;
-			schedule_timeout(HZ);
-			current->state = TASK_RUNNING;
-		}
-		kenvctrld_task = NULL;
-	}
+	kthread_stop(kenvctrld_task);
 
 	tp = all_bbc_temps;
 	while (tp != NULL) {
=== drivers/sbus/char/envctrl.c 1.18 vs edited ==--- 1.18/drivers/sbus/char/envctrl.c	2003-10-07 10:17:34 +02:00
+++ edited/drivers/sbus/char/envctrl.c	2004-06-14 13:32:58 +02:00
@@ -30,6 +30,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
+#include <linux/kthread.h>
 
 #include <asm/ebus.h>
 #include <asm/uaccess.h>
@@ -1027,19 +1028,10 @@
 
 	poll_interval = 5 * HZ; /* TODO env_mon_interval */
 
-	daemonize("kenvctrld");
-	allow_signal(SIGKILL);
-
-	kenvctrld_task = current;
-
 	printk(KERN_INFO "envctrl: %s starting...\n", current->comm);
-	for (;;) {
-		current->state = TASK_INTERRUPTIBLE;
-		schedule_timeout(poll_interval);
-
-		if(signal_pending(current))
-			break;
 
+	set_current_state(TASK_INTERRUPTIBLE);
+	while (!kthread_should_stop()) {
 		for (whichcpu = 0; whichcpu < ENVCTRL_MAX_CPU; ++whichcpu) {
 			if (0 < envctrl_read_cpu_info(whichcpu, cputemp,
 						      ENVCTRL_CPUTEMP_MON,
@@ -1054,7 +1046,10 @@
 				}
 			}
 		}
+		set_current_state(TASK_INTERRUPTIBLE);
 	}
+	__set_current_state(TASK_RUNNING);
+
 	printk(KERN_INFO "envctrl: %s exiting...\n", current->comm);
 	return 0;
 }
@@ -1139,9 +1134,11 @@
 			i2c_childlist[i].addr, (0 = i) ? ("\n") : (" "));
 	}
 
-	err = kernel_thread(kenvctrld, NULL, CLONE_FS | CLONE_FILES);
-	if (err < 0)
+	kenvctrld_task = kthread_create(kenvctrld, NULL, "%s", "kenvctrld");
+	if (IS_ERR(kenvctrld_task)) {
+		err = PTR_ERR(kenvctrld_task);
 		goto out_deregister;
+	}
 
 	return 0;
 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-06-15 18:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-15 17:29 sparc env monitorin vs tasklist_lock Christoph Hellwig
2004-06-15 17:42 ` Ben Collins
2004-06-15 18:04 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.