| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1153942954.3381.50.camel@laptopd505.fenrus.org>
Date: Wed, 26 Jul 2006 21:42:34 +0200
From: Arjan van de Ven <arjan@...ux.intel.com>
To: Linus Torvalds <torvalds@...l.org>
Cc: Dave Jones <davej@...hat.com>, Ingo Molnar <mingo@...e.hu>,
Chuck Ebbert <76306.1226@...puserve.com>,
Ashok Raj <ashok.raj@...el.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...l.org>
Subject: Re: [patch] Reorganize the cpufreq cpu hotplug locking to not be
totally bizare
On Wed, 2006-07-26 at 10:09 -0700, Linus Torvalds wrote:
> It looked sensible to me too, although it still shows some "Lukewarm IQ"
> notices for the ondemand driver:
>
> Lukewarm IQ detected in hotplug locking
> BUG: warning at kernel/cpu.c:38/lock_cpu_hotplug()
> [<c0103d07>] show_trace+0xd/0x10
> [<c01042ec>] dump_stack+0x19/0x1b
> [<c013778d>] lock_cpu_hotplug+0x43/0x69
> [<c012f9df>] __create_workqueue+0x52/0x11f
> [<df0ec34b>] cpufreq_governor_dbs+0x9f/0x2bd [cpufreq_ondemand]
ok so this is messy. ondemand wants to create a workqueue, but this
function is called (after my patch) with the lock_cpu_hotplug() already
held, and all the workqueue functions take the lock_cpu_hotplug
themselves (looking at that code I'm not 100% convinced it's quite right
how it does that, but that's a separate matter). Before my first patch
it wasn't doing that, but that situation was even nastier; there is a
lock that is taken in the caller, and the lock_cpu_hotplug() lock
*really* wants to nest outside that lock ;(
As a quick hack I made non-lock_cpu_hotplug()'ing versions of the 3 key
workqueue functions (patch below). It works, it's correct, it's just so
ugly that I'm almost too ashamed to post it. I haven't found a better
solution yet though... time to take a step back I suppose.
Index: linux-2.6.18-rc2-git5/include/linux/workqueue.h
===================================================================
--- linux-2.6.18-rc2-git5.orig/include/linux/workqueue.h
+++ linux-2.6.18-rc2-git5/include/linux/workqueue.h
@@ -55,17 +55,20 @@ struct execute_work {
} while (0)
extern struct workqueue_struct *__create_workqueue(const char *name,
- int singlethread);
-#define create_workqueue(name) __create_workqueue((name), 0)
-#define create_singlethread_workqueue(name) __create_workqueue((name), 1)
+ int singlethread, int locked);
+#define create_workqueue(name) __create_workqueue((name), 0, 0)
+#define create_workqueue_cpulocked(name) __create_workqueue((name), 0, 1)
+#define create_singlethread_workqueue(name) __create_workqueue((name), 1, 0)
extern void destroy_workqueue(struct workqueue_struct *wq);
+extern void destroy_workqueue_cpulock(struct workqueue_struct *wq);
extern int FASTCALL(queue_work(struct workqueue_struct *wq, struct work_struct *work));
extern int FASTCALL(queue_delayed_work(struct workqueue_struct *wq, struct work_struct *work, unsigned long delay));
extern int queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
struct work_struct *work, unsigned long delay);
extern void FASTCALL(flush_workqueue(struct workqueue_struct *wq));
+extern void FASTCALL(__flush_workqueue(struct workqueue_struct *wq));
extern int FASTCALL(schedule_work(struct work_struct *work));
extern int FASTCALL(schedule_delayed_work(struct work_struct *work, unsigned long delay));
Index: linux-2.6.18-rc2-git5/kernel/workqueue.c
===================================================================
--- linux-2.6.18-rc2-git5.orig/kernel/workqueue.c
+++ linux-2.6.18-rc2-git5/kernel/workqueue.c
@@ -307,6 +307,22 @@ void fastcall flush_workqueue(struct wor
}
EXPORT_SYMBOL_GPL(flush_workqueue);
+void fastcall __flush_workqueue(struct workqueue_struct *wq)
+{
+ might_sleep();
+
+ if (is_single_threaded(wq)) {
+ /* Always use first cpu's area. */
+ flush_cpu_workqueue(per_cpu_ptr(wq->cpu_wq, singlethread_cpu));
+ } else {
+ int cpu;
+
+ for_each_online_cpu(cpu)
+ flush_cpu_workqueue(per_cpu_ptr(wq->cpu_wq, cpu));
+ }
+}
+EXPORT_SYMBOL_GPL(__flush_workqueue);
+
static struct task_struct *create_workqueue_thread(struct workqueue_struct *wq,
int cpu)
{
@@ -333,7 +349,7 @@ static struct task_struct *create_workqu
}
struct workqueue_struct *__create_workqueue(const char *name,
- int singlethread)
+ int singlethread, int locked)
{
int cpu, destroy = 0;
struct workqueue_struct *wq;
@@ -351,7 +367,8 @@ struct workqueue_struct *__create_workqu
wq->name = name;
/* We don't need the distraction of CPUs appearing and vanishing. */
- lock_cpu_hotplug();
+ if (!locked)
+ lock_cpu_hotplug();
if (singlethread) {
INIT_LIST_HEAD(&wq->list);
p = create_workqueue_thread(wq, singlethread_cpu);
@@ -372,7 +389,8 @@ struct workqueue_struct *__create_workqu
destroy = 1;
}
}
- unlock_cpu_hotplug();
+ if (!locked)
+ unlock_cpu_hotplug();
/*
* Was there any error during startup? If yes then clean up:
@@ -400,14 +418,17 @@ static void cleanup_workqueue_thread(str
kthread_stop(p);
}
-void destroy_workqueue(struct workqueue_struct *wq)
+static void __destroy_workqueue(struct workqueue_struct *wq, int locked)
{
int cpu;
- flush_workqueue(wq);
/* We don't need the distraction of CPUs appearing and vanishing. */
- lock_cpu_hotplug();
+ if (!locked)
+ lock_cpu_hotplug();
+
+ __flush_workqueue(wq);
+
if (is_single_threaded(wq))
cleanup_workqueue_thread(wq, singlethread_cpu);
else {
@@ -417,11 +438,22 @@ void destroy_workqueue(struct workqueue_
list_del(&wq->list);
spin_unlock(&workqueue_lock);
}
- unlock_cpu_hotplug();
+ if (!locked)
+ unlock_cpu_hotplug();
free_percpu(wq->cpu_wq);
kfree(wq);
}
+
+void destroy_workqueue(struct workqueue_struct *wq)
+{
+ __destroy_workqueue(wq, 0);
+}
EXPORT_SYMBOL_GPL(destroy_workqueue);
+void destroy_workqueue_cpulock(struct workqueue_struct *wq)
+{
+ __destroy_workqueue(wq, 1);
+}
+EXPORT_SYMBOL_GPL(destroy_workqueue_cpulock);
static struct workqueue_struct *keventd_wq;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists