Messages in this thread Patch in this message | | | Date | Wed, 18 Aug 2010 14:50:25 +0200 (CEST) | From | Stefan Richter <> | Subject | Re: lockdep false positive? -- firewire-core transaction timer vs. scsi-core host lock |
| |
Clemens Ladisch wrote: > firewire: core: do not use del_timer_sync() in interrupt context > > Because we might be in interrupt context, replace del_timer_sync() with > del_timer().
OK. And thanks for the pointers into the locking guide.
> If the timer is already running, we know that it will > clean up the transaction, so we do not need to do any further processing > in the normal transaction handler. > > Many thanks to Yong Zhang for diagnosing this. > > Reported-by: Stefan Richter <stefanr@s5r6.in-berlin.de> > Signed-off-by: Clemens Ladisch <clemens@ladisch.de> > --- > drivers/firewire/core-transaction.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > --- a/drivers/firewire/core-transaction.c > +++ b/drivers/firewire/core-transaction.c > @@ -81,6 +81,8 @@ static int close_transaction(struct fw_transaction *transaction, > spin_lock_irqsave(&card->lock, flags); > list_for_each_entry(t, &card->transaction_list, link) { > if (t == transaction) { > + if (!del_timer(&t->split_timeout_timer)) > + goto timed_out;
+ if (!del_timer(&t->split_timeout_timer)) { + spin_unlock_irqrestore(&card->lock, flags); + goto timed_out; + }
> list_del_init(&t->link); > card->tlabel_mask &= ~(1ULL << t->tlabel); > break; > @@ -89,11 +91,11 @@ static int close_transaction(struct fw_transaction *transaction, > spin_unlock_irqrestore(&card->lock, flags); > > if (&t->link != &card->transaction_list) { > - del_timer_sync(&t->split_timeout_timer); > t->callback(card, rcode, NULL, 0, t->callback_data); > return 0; > } > > +timed_out: > return -ENOENT; > } > > @@ -921,6 +923,8 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) > spin_lock_irqsave(&card->lock, flags); > list_for_each_entry(t, &card->transaction_list, link) { > if (t->node_id == source && t->tlabel == tlabel) { > + if (!del_timer(&t->split_timeout_timer)) > + goto timed_out;
Ditto.
> list_del_init(&t->link); > card->tlabel_mask &= ~(1ULL << t->tlabel); > break; > @@ -929,6 +933,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) > spin_unlock_irqrestore(&card->lock, flags); > > if (&t->link == &card->transaction_list) { > +timed_out: > fw_notify("Unsolicited response (source %x, tlabel %x)\n", > source, tlabel); > return; > @@ -963,8 +968,6 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p) > break; > } > > - del_timer_sync(&t->split_timeout_timer); > - > /* > * The response handler may be executed while the request handler > * is still pending. Cancel the request handler.
Shall I commit with the added spin_unlocks? Or do you prefer to send an update? -- Stefan Richter -=====-==-=- =--- =--=- http://arcgraph.de/sr/
| |