On Tuesday 04 July 2006 00:45, you wrote:
> +static void ams_handle_irq(void *data)
> +{
> + enum ams_irq irq = *((enum ams_irq *)data);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ams.irq_lock, flags);
> +
> + ams.worker_irqs |= irq;
> + schedule_work(&ams.worker);
> +
> + spin_unlock_irqrestore(&ams.irq_lock, flags);
> +}
I would say this is racy.
Locks should be added as shown below.
> +static void ams_worker(void *data)
> +{
unsigned long flags;
> + mutex_lock(&ams.lock);
> +
> + if (ams.has_device) {
spin_lock_irqsave(&ams.irq_lock, flags);
> + if (ams.worker_irqs & AMS_IRQ_FREEFALL) {
> + if (verbose)
> + printk(KERN_INFO "ams: freefall detected!\n");
> +
> + ams.worker_irqs &= ~AMS_IRQ_FREEFALL;
> + ams.clear_irq(AMS_IRQ_FREEFALL);
> + }
> +
> + if (ams.worker_irqs & AMS_IRQ_SHOCK) {
> + if (verbose)
> + printk(KERN_INFO "ams: shock detected!\n");
> +
> + ams.worker_irqs &= ~AMS_IRQ_SHOCK;
> + ams.clear_irq(AMS_IRQ_SHOCK);
> +
spin_unlock_irqrestore(&ams.irq_lock, flags);
> + }
> +
> + mutex_unlock(&ams.lock);
> +}
Otherwise an IRQ could trigger while the bottom half is
executing and corrupt the ams.worker_irqs field, because it
is modified non-atomically.
--
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
