lkml.org 
[lkml]   [2010]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 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/



\
 
 \ /
  Last update: 2010-08-18 14:53    [W:0.052 / U:0.708 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site