* [PATCH] cx88: Prevent general protection fault on rmmod
@ 2009-03-07 10:42 Jean Delvare
0 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-03-07 10:42 UTC (permalink / raw)
To: LMML; +Cc: Trent Piepho
When unloading the cx8800 driver I sometimes get a general protection
fault. Analysis revealed a race in cx88_ir_stop(). It can be solved by
using a delayed work instead of a timer for infrared input polling.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
Thanks to Trent's compatibility patches, we can go without the bunch of
#ifdef's my initial patch had.
linux/drivers/media/video/cx88/cx88-input.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
--- v4l-dvb.orig/linux/drivers/media/video/cx88/cx88-input.c 2009-03-05 10:36:23.000000000 +0100
+++ v4l-dvb/linux/drivers/media/video/cx88/cx88-input.c 2009-03-06 13:59:56.000000000 +0100
@@ -49,8 +49,7 @@ struct cx88_IR {
/* poll external decoder */
int polling;
- struct work_struct work;
- struct timer_list timer;
+ struct delayed_work work;
u32 gpio_addr;
u32 last_gpio;
u32 mask_keycode;
@@ -144,13 +143,6 @@ static void cx88_ir_handle_key(struct cx
}
}
-static void ir_timer(unsigned long data)
-{
- struct cx88_IR *ir = (struct cx88_IR *)data;
-
- schedule_work(&ir->work);
-}
-
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
static void cx88_ir_work(void *data)
#else
@@ -160,23 +152,22 @@ static void cx88_ir_work(struct work_str
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
struct cx88_IR *ir = data;
#else
- struct cx88_IR *ir = container_of(work, struct cx88_IR, work);
+ struct cx88_IR *ir = container_of(work, struct cx88_IR, work.work);
#endif
cx88_ir_handle_key(ir);
- mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling));
+ schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
}
void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
{
if (ir->polling) {
- setup_timer(&ir->timer, ir_timer, (unsigned long)ir);
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
- INIT_WORK(&ir->work, cx88_ir_work, ir);
+ INIT_DELAYED_WORK(&ir->work, cx88_ir_work, ir);
#else
- INIT_WORK(&ir->work, cx88_ir_work);
+ INIT_DELAYED_WORK(&ir->work, cx88_ir_work);
#endif
- schedule_work(&ir->work);
+ schedule_delayed_work(&ir->work, 0);
}
if (ir->sampling) {
core->pci_irqmask |= PCI_INT_IR_SMPINT;
@@ -192,10 +183,8 @@ void cx88_ir_stop(struct cx88_core *core
core->pci_irqmask &= ~PCI_INT_IR_SMPINT;
}
- if (ir->polling) {
- del_timer_sync(&ir->timer);
- flush_scheduled_work();
- }
+ if (ir->polling)
+ cancel_delayed_work_sync(&ir->work);
}
/* ---------------------------------------------------------------------- */
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] cx88: Prevent general protection fault on rmmod
@ 2009-03-05 9:38 Jean Delvare
2009-03-05 9:43 ` Trent Piepho
0 siblings, 1 reply; 4+ messages in thread
From: Jean Delvare @ 2009-03-05 9:38 UTC (permalink / raw)
To: LMML
From: Jean Delvare <khali@linux-fr.org>
Subject: cx88: Prevent general protection fault on rmmod
When unloading the cx8800 driver I sometimes get a general protection
fault. Analysis revealed a race in cx88_ir_stop(). It can be solved by
using a delayed work instead of a timer for infrared input polling.
This fixes kernel.org bug #12802.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
linux/drivers/media/video/cx88/cx88-input.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
--- v4l-dvb.orig/linux/drivers/media/video/cx88/cx88-input.c 2009-03-04 09:52:20.000000000 +0100
+++ v4l-dvb/linux/drivers/media/video/cx88/cx88-input.c 2009-03-04 19:03:17.000000000 +0100
@@ -49,8 +49,12 @@ struct cx88_IR {
/* poll external decoder */
int polling;
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
struct work_struct work;
struct timer_list timer;
+#else
+ struct delayed_work work;
+#endif
u32 gpio_addr;
u32 last_gpio;
u32 mask_keycode;
@@ -144,6 +148,7 @@ static void cx88_ir_handle_key(struct cx
}
}
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
static void ir_timer(unsigned long data)
{
struct cx88_IR *ir = (struct cx88_IR *)data;
@@ -151,7 +156,6 @@ static void ir_timer(unsigned long data)
schedule_work(&ir->work);
}
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
static void cx88_ir_work(void *data)
#else
static void cx88_ir_work(struct work_struct *work)
@@ -160,23 +164,30 @@ static void cx88_ir_work(struct work_str
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
struct cx88_IR *ir = data;
#else
- struct cx88_IR *ir = container_of(work, struct cx88_IR, work);
+ struct delayed_work *dwork = container_of(work, struct delayed_work,
+ work);
+ struct cx88_IR *ir = container_of(dwork, struct cx88_IR, work);
#endif
cx88_ir_handle_key(ir);
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling));
+#else
+ schedule_delayed_work(dwork, msecs_to_jiffies(ir->polling));
+#endif
}
void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
{
if (ir->polling) {
- setup_timer(&ir->timer, ir_timer, (unsigned long)ir);
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
+ setup_timer(&ir->timer, ir_timer, (unsigned long)ir);
INIT_WORK(&ir->work, cx88_ir_work, ir);
+ schedule_work(&ir->work);
#else
- INIT_WORK(&ir->work, cx88_ir_work);
+ INIT_DELAYED_WORK(&ir->work, cx88_ir_work);
+ schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
#endif
- schedule_work(&ir->work);
}
if (ir->sampling) {
core->pci_irqmask |= PCI_INT_IR_SMPINT;
@@ -193,8 +204,12 @@ void cx88_ir_stop(struct cx88_core *core
}
if (ir->polling) {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
del_timer_sync(&ir->timer);
flush_scheduled_work();
+#else
+ cancel_delayed_work_sync(&ir->work);
+#endif
}
}
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] cx88: Prevent general protection fault on rmmod
2009-03-05 9:38 Jean Delvare
@ 2009-03-05 9:43 ` Trent Piepho
2009-03-05 13:48 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Trent Piepho @ 2009-03-05 9:43 UTC (permalink / raw)
To: Jean Delvare; +Cc: LMML
On Thu, 5 Mar 2009, Jean Delvare wrote:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
> struct work_struct work;
> struct timer_list timer;
> +#else
> + struct delayed_work work;
> +#endif
You don't need this compat stuff. compat.h will take are of it for you.
Just code it like you would for the latest kernel. The only thing you need
to worry about is the way the third argument of the work function went
away, but the ifdef that's already there takes care of it.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cx88: Prevent general protection fault on rmmod
2009-03-05 9:43 ` Trent Piepho
@ 2009-03-05 13:48 ` Jean Delvare
0 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-03-05 13:48 UTC (permalink / raw)
To: Trent Piepho; +Cc: LMML
On Thu, 5 Mar 2009 01:43:55 -0800 (PST), Trent Piepho wrote:
> On Thu, 5 Mar 2009, Jean Delvare wrote:
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
> > struct work_struct work;
> > struct timer_list timer;
> > +#else
> > + struct delayed_work work;
> > +#endif
>
> You don't need this compat stuff. compat.h will take are of it for you.
> Just code it like you would for the latest kernel. The only thing you need
> to worry about is the way the third argument of the work function went
> away, but the ifdef that's already there takes care of it.
OK, thanks. I'll resend updated patches soon.
--
Jean Delvare
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-07 10:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-07 10:42 [PATCH] cx88: Prevent general protection fault on rmmod Jean Delvare
-- strict thread matches above, loose matches on Subject: below --
2009-03-05 9:38 Jean Delvare
2009-03-05 9:43 ` Trent Piepho
2009-03-05 13:48 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox