Hi Oliver
On Mon, Nov 14, 2011 at 8:54 AM, Oliver Neukum <oneukum@xxxxxxx> wrote:
> Am Sonntag, 13. November 2011, 14:27:07 schrieb David Herrmann:
>> +/* Deinitializes the extension driver of a wiimote */
>> +void wiiext_deinit(struct wiimote_data *wdata)
>> +{
>> + struct wiimote_ext *ext = wdata->ext;
>> + unsigned long flags;
>> +
>> + if (!ext)
>> + return;
>> +
>> + spin_lock_irqsave(&wdata->state.lock, flags);
>> + wdata->ext = NULL;
>> + spin_unlock_irqrestore(&wdata->state.lock, flags);
>> +
>> + cancel_work_sync(&ext->worker);
>> + kfree(ext);
>> +}
>
>
> Are you sure you are doing this in the correct order?
> You are needlessly leaving a window where wdate->ext is NULL
> but the worker may run.
Thanks for reviewing.
I am only removing the wdata->ext relationship here. This is not used
by the worker at all. I never access wdata->ext in this module without
taking the spinlock and without checking it for NULL.
The worker gets "ext" directy and can still access ext->wdata even if
ext->wdata->ext is NULL. This simplifies the locking a lot.
I also know that wdata lives longer than wdata->ext so I cannot find
any race condition here. I have checked all four entry points of the
extension-module and they all seem fine here.
As long as wdata->ext is non-NULL, wiiext_schedule may be called. So I
first need to clear wdata->ext before I kill the worker. Otherwise the
worker may be respawned before I reset wdata->ext ;)
> Regards
> Oliver
Thank you
David
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
