David Milburn wrote: > Hello, > > Shouldn't we synchronize ahci_sw_activity and ahci_sw_activity_blink? > Isn't it possible that __run_timers has detached the timer so it is > no longer pending and it is in the process of running the timer func > on one cpu, and ahci_sw_activity is running on another cpu and may > see no timer pending so both ahci_sw_activity and ahci_sw_activity_blink > modify the timeout. Also, ahci_sw_activity is incrementing > emp->activity and ahci_sw_activity_blink is reading this value. > Since ap->lock is already being held when ahci_sw_activity runs > shouldn't ahci_sw_activity_blink grab this lock to better sync > between the two? > > Thanks, > David > > drivers/ata/ahci.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index 2e1a7cb..1573ce3 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -1227,6 +1227,7 @@ static void ahci_sw_activity_blink(unsigned long arg) > * toggle state of LED and reset timer. If not, > * turn LED to desired idle state. > */ > + spin_lock(ap->lock); You'll need to use spin_lock_irqsave and unlock_irqrestore. Other than that, nice catch. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
