diff options
author | Stas Sergeev <stsp@aknet.ru> | 2008-05-20 05:47:29 -0400 |
---|---|---|
committer | Takashi Iwai <tiwai@suse.de> | 2008-05-20 05:56:24 -0400 |
commit | 4b7afb0d0d23b298a7e6d30eaba0679449542d2e (patch) | |
tree | beeff813fbb8d6dcffd13aefde4f43ccf9dc6f8e | |
parent | 8033c6e9736c29cce5f0d0abbca9a44dffb20c39 (diff) |
snd-pcsp: use HRTIMER_CB_SOFTIRQ
Change HRTIMER_CB_IRQSAFE to HRTIMER_CB_SOFTIRQ,
as suggested by Thomas Gleixner.
That solves the lock dependancy reported in
Bug #10701.
That also allows to call hrtimer_start()
directly, tasklet "stupid hack" removed.
Signed-off-by: Stas Sergeev <stsp@aknet.ru>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
-rw-r--r-- | sound/drivers/pcsp/pcsp.c | 2 | ||||
-rw-r--r-- | sound/drivers/pcsp/pcsp_lib.c | 37 |
2 files changed, 5 insertions, 34 deletions
diff --git a/sound/drivers/pcsp/pcsp.c b/sound/drivers/pcsp/pcsp.c index 54a1f9036c6..1899cf0685b 100644 --- a/sound/drivers/pcsp/pcsp.c +++ b/sound/drivers/pcsp/pcsp.c | |||
@@ -96,7 +96,7 @@ static int __devinit snd_card_pcsp_probe(int devnum, struct device *dev) | |||
96 | return -EINVAL; | 96 | return -EINVAL; |
97 | 97 | ||
98 | hrtimer_init(&pcsp_chip.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); | 98 | hrtimer_init(&pcsp_chip.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); |
99 | pcsp_chip.timer.cb_mode = HRTIMER_CB_IRQSAFE; | 99 | pcsp_chip.timer.cb_mode = HRTIMER_CB_SOFTIRQ; |
100 | pcsp_chip.timer.function = pcsp_do_timer; | 100 | pcsp_chip.timer.function = pcsp_do_timer; |
101 | 101 | ||
102 | card = snd_card_new(index, id, THIS_MODULE, 0); | 102 | card = snd_card_new(index, id, THIS_MODULE, 0); |
diff --git a/sound/drivers/pcsp/pcsp_lib.c b/sound/drivers/pcsp/pcsp_lib.c index 7ad4a1534b2..e341f3f83b6 100644 --- a/sound/drivers/pcsp/pcsp_lib.c +++ b/sound/drivers/pcsp/pcsp_lib.c | |||
@@ -9,7 +9,6 @@ | |||
9 | #include <linux/module.h> | 9 | #include <linux/module.h> |
10 | #include <linux/moduleparam.h> | 10 | #include <linux/moduleparam.h> |
11 | #include <sound/pcm.h> | 11 | #include <sound/pcm.h> |
12 | #include <linux/interrupt.h> | ||
13 | #include <asm/io.h> | 12 | #include <asm/io.h> |
14 | #include "pcsp.h" | 13 | #include "pcsp.h" |
15 | 14 | ||
@@ -20,34 +19,8 @@ MODULE_PARM_DESC(nforce_wa, "Apply NForce chipset workaround " | |||
20 | 19 | ||
21 | #define DMIX_WANTS_S16 1 | 20 | #define DMIX_WANTS_S16 1 |
22 | 21 | ||
23 | static void pcsp_start_timer(unsigned long dummy) | ||
24 | { | ||
25 | hrtimer_start(&pcsp_chip.timer, ktime_set(0, 0), HRTIMER_MODE_REL); | ||
26 | } | ||
27 | |||
28 | /* | ||
29 | * We need the hrtimer_start as a tasklet to avoid | ||
30 | * the nasty locking problem. :( | ||
31 | * The problem: | ||
32 | * - The timer handler is called with the cpu_base->lock | ||
33 | * already held by hrtimer code. | ||
34 | * - snd_pcm_period_elapsed() takes the | ||
35 | * substream->self_group.lock. | ||
36 | * So far so good. | ||
37 | * But the snd_pcsp_trigger() is called with the | ||
38 | * substream->self_group.lock held, and it calls | ||
39 | * hrtimer_start(), which takes the cpu_base->lock. | ||
40 | * You see the problem. We have the code pathes | ||
41 | * which take two locks in a reverse order. This | ||
42 | * can deadlock and the lock validator complains. | ||
43 | * The only solution I could find was to move the | ||
44 | * hrtimer_start() into a tasklet. -stsp | ||
45 | */ | ||
46 | static DECLARE_TASKLET(pcsp_start_timer_tasklet, pcsp_start_timer, 0); | ||
47 | |||
48 | enum hrtimer_restart pcsp_do_timer(struct hrtimer *handle) | 22 | enum hrtimer_restart pcsp_do_timer(struct hrtimer *handle) |
49 | { | 23 | { |
50 | unsigned long flags; | ||
51 | unsigned char timer_cnt, val; | 24 | unsigned char timer_cnt, val; |
52 | int fmt_size, periods_elapsed; | 25 | int fmt_size, periods_elapsed; |
53 | u64 ns; | 26 | u64 ns; |
@@ -66,9 +39,7 @@ enum hrtimer_restart pcsp_do_timer(struct hrtimer *handle) | |||
66 | return HRTIMER_RESTART; | 39 | return HRTIMER_RESTART; |
67 | } | 40 | } |
68 | 41 | ||
69 | /* hrtimer calls us from both hardirq and softirq contexts, | 42 | spin_lock_irq(&chip->substream_lock); |
70 | * so irqsave :( */ | ||
71 | spin_lock_irqsave(&chip->substream_lock, flags); | ||
72 | /* Takashi Iwai says regarding this extra lock: | 43 | /* Takashi Iwai says regarding this extra lock: |
73 | 44 | ||
74 | If the irq handler handles some data on the DMA buffer, it should | 45 | If the irq handler handles some data on the DMA buffer, it should |
@@ -139,7 +110,7 @@ enum hrtimer_restart pcsp_do_timer(struct hrtimer *handle) | |||
139 | chip->period_ptr %= buffer_bytes; | 110 | chip->period_ptr %= buffer_bytes; |
140 | } | 111 | } |
141 | 112 | ||
142 | spin_unlock_irqrestore(&chip->substream_lock, flags); | 113 | spin_unlock_irq(&chip->substream_lock); |
143 | 114 | ||
144 | if (!atomic_read(&chip->timer_active)) | 115 | if (!atomic_read(&chip->timer_active)) |
145 | return HRTIMER_NORESTART; | 116 | return HRTIMER_NORESTART; |
@@ -153,7 +124,7 @@ enum hrtimer_restart pcsp_do_timer(struct hrtimer *handle) | |||
153 | exit_nr_unlock2: | 124 | exit_nr_unlock2: |
154 | snd_pcm_stream_unlock(substream); | 125 | snd_pcm_stream_unlock(substream); |
155 | exit_nr_unlock1: | 126 | exit_nr_unlock1: |
156 | spin_unlock_irqrestore(&chip->substream_lock, flags); | 127 | spin_unlock_irq(&chip->substream_lock); |
157 | return HRTIMER_NORESTART; | 128 | return HRTIMER_NORESTART; |
158 | } | 129 | } |
159 | 130 | ||
@@ -174,7 +145,7 @@ static void pcsp_start_playing(struct snd_pcsp *chip) | |||
174 | atomic_set(&chip->timer_active, 1); | 145 | atomic_set(&chip->timer_active, 1); |
175 | chip->thalf = 0; | 146 | chip->thalf = 0; |
176 | 147 | ||
177 | tasklet_schedule(&pcsp_start_timer_tasklet); | 148 | hrtimer_start(&pcsp_chip.timer, ktime_set(0, 0), HRTIMER_MODE_REL); |
178 | } | 149 | } |
179 | 150 | ||
180 | static void pcsp_stop_playing(struct snd_pcsp *chip) | 151 | static void pcsp_stop_playing(struct snd_pcsp *chip) |