Re: [PATCH 1/3] [AF_IUCV/IUCV]: smp_call_function deadlock

2007-04-28 Thread David Miller
From: Frank Pavlic <[EMAIL PROTECTED]>
Date: Thu, 19 Apr 2007 11:11:45 +0200

> From: Martin Schwidefsky <[EMAIL PROTECTED]>
> From: Heiko Carstens <[EMAIL PROTECTED]>
> From: Ursula Braun <[EMAIL PROTECTED]>
> 
> Calling smp_call_function can lead to a deadlock if it is called
> from tasklet context. 
> Fixing this deadlock requires to move the smp_call_function from the
> tasklet context to a work queue. To do that queue the path pending
> interrupts to a separate list and move the path cleanup out of
> iucv_path_sever to iucv_path_connect and iucv_path_pending.
> This creates a new requirement for iucv_path_connect: it may not be
> called from tasklet context anymore. 
> Also fixed compile problem for CONFIG_HOTPLUG_CPU=n and
> another one when walking the cpu_online mask. When doing this, 
> we must disable cpu hotplug.
> 
> Signed-off-by: Frank Pavlic <[EMAIL PROTECTED]>
> Signed-off-by: Martin Schwidefsky <[EMAIL PROTECTED]>

Applied, except I had to kick out the HOTPLUT_CPU ifdef removal
since that was already done by f8a6d97043f9adc25889876b681998b77f543bfa

Thanks.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] [AF_IUCV/IUCV]: smp_call_function deadlock

2007-04-19 Thread Frank Pavlic
From: Martin Schwidefsky <[EMAIL PROTECTED]>
From: Heiko Carstens <[EMAIL PROTECTED]>
From: Ursula Braun <[EMAIL PROTECTED]>

Calling smp_call_function can lead to a deadlock if it is called
from tasklet context. 
Fixing this deadlock requires to move the smp_call_function from the
tasklet context to a work queue. To do that queue the path pending
interrupts to a separate list and move the path cleanup out of
iucv_path_sever to iucv_path_connect and iucv_path_pending.
This creates a new requirement for iucv_path_connect: it may not be
called from tasklet context anymore. 
Also fixed compile problem for CONFIG_HOTPLUG_CPU=n and
another one when walking the cpu_online mask. When doing this, 
we must disable cpu hotplug.

Signed-off-by: Frank Pavlic <[EMAIL PROTECTED]>
Signed-off-by: Martin Schwidefsky <[EMAIL PROTECTED]>
---

 include/net/iucv/iucv.h |2
 net/iucv/iucv.c |  207 ++--
 2 files changed, 133 insertions(+), 76 deletions(-)

diff --git a/include/net/iucv/iucv.h b/include/net/iucv/iucv.h
index 746e741..fd70adb 100644
--- a/include/net/iucv/iucv.h
+++ b/include/net/iucv/iucv.h
@@ -16,7 +16,7 @@
  * completed a register, it can exploit the other functions.
  * For furthur reference on all IUCV functionality, refer to the
  * CP Programming Services book, also available on the web thru
- * www.ibm.com/s390/vm/pubs, manual # SC24-5760
+ * www.vm.ibm.com/pubs, manual # SC24-6084
  *
  * Definition of Return Codes
  * - All positive return codes including zero are reflected back
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c
index 1b10d57..903bdb6 100644
--- a/net/iucv/iucv.c
+++ b/net/iucv/iucv.c
@@ -90,20 +90,43 @@ struct iucv_irq_data {
u32 res2[8];
 };
 
-struct iucv_work {
+struct iucv_irq_list {
struct list_head list;
struct iucv_irq_data data;
 };
 
-static LIST_HEAD(iucv_work_queue);
-static DEFINE_SPINLOCK(iucv_work_lock);
-
 static struct iucv_irq_data *iucv_irq_data;
 static cpumask_t iucv_buffer_cpumask = CPU_MASK_NONE;
 static cpumask_t iucv_irq_cpumask = CPU_MASK_NONE;
 
-static void iucv_tasklet_handler(unsigned long);
-static DECLARE_TASKLET(iucv_tasklet, iucv_tasklet_handler,0);
+/*
+ * Queue of interrupt buffers lock for delivery via the tasklet
+ * (fast but can't call smp_call_function).
+ */
+static LIST_HEAD(iucv_task_queue);
+
+/*
+ * The tasklet for fast delivery of iucv interrupts.
+ */
+static void iucv_tasklet_fn(unsigned long);
+static DECLARE_TASKLET(iucv_tasklet, iucv_tasklet_fn,0);
+
+/*
+ * Queue of interrupt buffers for delivery via a work queue
+ * (slower but can call smp_call_function).
+ */
+static LIST_HEAD(iucv_work_queue);
+
+/*
+ * The work element to deliver path pending interrupts.
+ */
+static void iucv_work_fn(struct work_struct *work);
+static DECLARE_WORK(iucv_work, iucv_work_fn);
+
+/*
+ * Spinlock protecting task and work queue.
+ */
+static DEFINE_SPINLOCK(iucv_queue_lock);
 
 enum iucv_command_codes {
IUCV_QUERY = 0,
@@ -147,10 +170,10 @@ static unsigned long iucv_max_pathid;
 static DEFINE_SPINLOCK(iucv_table_lock);
 
 /*
- * iucv_tasklet_cpu: contains the number of the cpu executing the tasklet.
- * Needed for iucv_path_sever called from tasklet.
+ * iucv_active_cpu: contains the number of the cpu executing the tasklet
+ * or the work handler. Needed for iucv_path_sever called from tasklet.
  */
-static int iucv_tasklet_cpu = -1;
+static int iucv_active_cpu = -1;
 
 /*
  * Mutex and wait queue for iucv_register/iucv_unregister.
@@ -449,17 +472,19 @@ static void iucv_setmask_mp(void)
 {
int cpu;
 
+   preempt_disable();
for_each_online_cpu(cpu)
/* Enable all cpus with a declared buffer. */
if (cpu_isset(cpu, iucv_buffer_cpumask) &&
!cpu_isset(cpu, iucv_irq_cpumask))
smp_call_function_on(iucv_allow_cpu, NULL, 0, 1, cpu);
+   preempt_enable();
 }
 
 /**
  * iucv_setmask_up
  *
- * Allow iucv interrupts on a single cpus.
+ * Allow iucv interrupts on a single cpu.
  */
 static void iucv_setmask_up(void)
 {
@@ -493,8 +518,10 @@ static int iucv_enable(void)
goto out;
/* Declare per cpu buffers. */
rc = -EIO;
+   preempt_disable();
for_each_online_cpu(cpu)
smp_call_function_on(iucv_declare_cpu, NULL, 0, 1, cpu);
+   preempt_enable();
if (cpus_empty(iucv_buffer_cpumask))
/* No cpu could declare an iucv buffer. */
goto out_path;
@@ -519,7 +546,6 @@ static void iucv_disable(void)
kfree(iucv_path_table);
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
 static int __cpuinit iucv_cpu_notify(struct notifier_block *self,
 unsigned long action, void *hcpu)
 {
@@ -565,7 +591,6 @@ static int __cpuinit iucv_cpu_notify(struct notifier_block 
*self,
 static struct notifier_block iucv_cpu_notifier = {
.notifier_call = iucv_cpu_notify,
 };
-#endif
 
 /**