Am Donnerstag, 19. April 2012, 17:29:48 schrieb Greg KH:
> On Thu, Apr 19, 2012 at 01:11:44PM +0200, Oliver Neukum wrote:
> > Hi,
> >
> > I've noticed quite some uses of del_timer_sync() in interrupt
> > in the uwb code. The driver combines reference counting with
> > timers, but doesn't grab a reference for timers it starts.
> > I'd like to fix this by introducing references for timers.
> > Any objections?
>
> What do you mean by "references for timers"? Are the timers that are
> started cleaned up when needed? Or are you finding problems with it?
It does things like this:
/**
* The entity that reads from the device notification/event channel has
* detected an error.
*
* @rc: UWB Radio Controller
* @error: Errno error code
*
*/
void uwb_rc_neh_error(struct uwb_rc *rc, int error)
{
struct uwb_rc_neh *neh;
unsigned long flags;
for (;;) {
spin_lock_irqsave(&rc->neh_lock, flags);
if (list_empty(&rc->neh_list)) {
spin_unlock_irqrestore(&rc->neh_lock, flags);
break;
}
neh = list_first_entry(&rc->neh_list, struct uwb_rc_neh, list_node);
__uwb_rc_neh_rm(rc, neh);
spin_unlock_irqrestore(&rc->neh_lock, flags);
del_timer_sync(&neh->timer);
uwb_rc_neh_cb(neh, NULL, error);
}
}
It is called in interrupt, yet uses del_timer_sync()
The timer drops a reference:
static void uwb_rc_neh_cb(struct uwb_rc_neh *neh, struct uwb_rceb *rceb, size_t size)
{
(*neh->cb)(neh->rc, neh->arg, rceb, size);
uwb_rc_neh_put(neh);
}
static void uwb_rc_neh_timer(unsigned long arg)
{
struct uwb_rc_neh *neh = (struct uwb_rc_neh *)arg;
struct uwb_rc *rc = neh->rc;
unsigned long flags;
spin_lock_irqsave(&rc->neh_lock, flags);
if (neh->context)
__uwb_rc_neh_rm(rc, neh);
else
neh = NULL;
spin_unlock_irqrestore(&rc->neh_lock, flags);
if (neh)
uwb_rc_neh_cb(neh, NULL, -ETIMEDOUT);
}
This looks quite fishy.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
