firewire: core: use separate timeout for each transaction

Using a single timeout for all transaction that need to be flushed does
not work if the submission of new transactions can defer the timeout
indefinitely into the future.  We need to have timeouts that do not
change due to other transactions; the simplest way to do this is with a
separate timer for each transaction.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (+ one lockdep annotation)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 42cf911b73cfad200e979f12883dd4c035ee0dab..9dcb30466ec0a22a11092decb65474e7bdcc5191 100644 (file)
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -30,7 +30,6 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
-#include <linux/timer.h>
 #include <linux/workqueue.h>
 
 #include <asm/atomic.h>
@@ -408,13 +407,6 @@ static void fw_card_bm_work(struct work_struct *work)
        fw_card_put(card);
 }
 
-static void flush_timer_callback(unsigned long data)
-{
-       struct fw_card *card = (struct fw_card *)data;
-
-       fw_flush_transactions(card);
-}
-
 void fw_card_initialize(struct fw_card *card,
                        const struct fw_card_driver *driver,
                        struct device *device)
@@ -433,8 +425,6 @@ void fw_card_initialize(struct fw_card *card,
        init_completion(&card->done);
        INIT_LIST_HEAD(&card->transaction_list);
        spin_lock_init(&card->lock);
-       setup_timer(&card->flush_timer,
-                   flush_timer_callback, (unsigned long)card);
 
        card->local_node = NULL;
 
@@ -559,7 +549,6 @@ void fw_core_remove_card(struct fw_card *card)
        wait_for_completion(&card->done);
 
        WARN_ON(!list_empty(&card->transaction_list));
-       del_timer_sync(&card->flush_timer);
 }
 EXPORT_SYMBOL(fw_core_remove_card);
 

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 901669876c253e78518c91331ac5a37c6786827b..fdc33ff06dc11c536b080797d1ba4a81482711b0 100644 (file)
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -81,7 +81,7 @@ 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) {
-                       list_del(&t->link);
+                       list_del_init(&t->link);
                        card->tlabel_mask &= ~(1ULL << t->tlabel);
                        break;
                }
@@ -89,6 +89,7 @@ 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;
        }
@@ -121,6 +122,31 @@ int fw_cancel_transaction(struct fw_card *card,
 }
 EXPORT_SYMBOL(fw_cancel_transaction);
 
+static void split_transaction_timeout_callback(unsigned long data)
+{
+       struct fw_transaction *t = (struct fw_transaction *)data;
+       struct fw_card *card = t->card;
+       unsigned long flags;
+
+       spin_lock_irqsave(&card->lock, flags);
+       if (list_empty(&t->link)) {
+               spin_unlock_irqrestore(&card->lock, flags);
+               return;
+       }
+       list_del(&t->link);
+       card->tlabel_mask &= ~(1ULL << t->tlabel);
+       spin_unlock_irqrestore(&card->lock, flags);
+
+       card->driver->cancel_packet(card, &t->packet);
+
+       /*
+        * At this point cancel_packet will never call the transaction
+        * callback, since we just took the transaction out of the list.
+        * So do it here.
+        */
+       t->callback(card, RCODE_CANCELLED, NULL, 0, t->callback_data);
+}
+
 static void transmit_complete_callback(struct fw_packet *packet,
                                       struct fw_card *card, int status)
 {
@@ -293,13 +319,6 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode,
        unsigned long flags;
        int tlabel;
 
-       /*
-        * Bump the flush timer up 100ms first of all so we
-        * don't race with a flush timer callback.
-        */
-
-       mod_timer(&card->flush_timer, jiffies + DIV_ROUND_UP(HZ, 10));
-
        /*
         * Allocate tlabel from the bitmap and put the transaction on
         * the list while holding the card spinlock.
@@ -316,6 +335,11 @@ void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode,
 
        t->node_id = destination_id;
        t->tlabel = tlabel;
+       t->card = card;
+       setup_timer(&t->split_timeout_timer,
+                   split_transaction_timeout_callback, (unsigned long)t);
+       /* FIXME: start this timer later, relative to t->timestamp */
+       mod_timer(&t->split_timeout_timer, jiffies + DIV_ROUND_UP(HZ, 10));
        t->callback = callback;
        t->callback_data = callback_data;
 
@@ -361,11 +385,13 @@ int fw_run_transaction(struct fw_card *card, int tcode, int destination_id,
        struct transaction_callback_data d;
        struct fw_transaction t;
 
+       init_timer_on_stack(&t.split_timeout_timer);
        init_completion(&d.done);
        d.payload = payload;
        fw_send_request(card, &t, tcode, destination_id, generation, speed,
                        offset, payload, length, transaction_callback, &d);
        wait_for_completion(&d.done);
+       destroy_timer_on_stack(&t.split_timeout_timer);
 
        return d.rcode;
 }
@@ -408,30 +434,6 @@ void fw_send_phy_config(struct fw_card *card,
        mutex_unlock(&phy_config_mutex);
 }
 
-void fw_flush_transactions(struct fw_card *card)
-{
-       struct fw_transaction *t, *next;
-       struct list_head list;
-       unsigned long flags;
-
-       INIT_LIST_HEAD(&list);
-       spin_lock_irqsave(&card->lock, flags);
-       list_splice_init(&card->transaction_list, &list);
-       card->tlabel_mask = 0;
-       spin_unlock_irqrestore(&card->lock, flags);
-
-       list_for_each_entry_safe(t, next, &list, link) {
-               card->driver->cancel_packet(card, &t->packet);
-
-               /*
-                * At this point cancel_packet will never call the
-                * transaction callback, since we just took all the
-                * transactions out of the list.  So do it here.
-                */
-               t->callback(card, RCODE_CANCELLED, NULL, 0, t->callback_data);
-       }
-}
-
 static struct fw_address_handler *lookup_overlapping_address_handler(
        struct list_head *list, unsigned long long offset, size_t length)
 {
@@ -841,7 +843,7 @@ 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) {
-                       list_del(&t->link);
+                       list_del_init(&t->link);
                        card->tlabel_mask &= ~(1ULL << t->tlabel);
                        break;
                }
@@ -883,6 +885,8 @@ 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.

diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index 7a9759bf68373d9c4f33baf527bb73c1e9b928e5..0ecfcd95f4c500f62389aa4085c68281b648cc7c 100644 (file)
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -220,7 +220,6 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *request);
 void fw_core_handle_response(struct fw_card *card, struct fw_packet *packet);
 void fw_fill_response(struct fw_packet *response, u32 *request_header,
                      int rcode, void *payload, size_t length);
-void fw_flush_transactions(struct fw_card *card);
 void fw_send_phy_config(struct fw_card *card,
                        int node_id, int generation, int gap_count);
 

diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index a527d73f99663f2c9e7ddfa0f38d6a77aaec0c10..72e2b8ac2a5a997b8d055e72d81d7358cddf6dce 100644 (file)
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -87,7 +87,6 @@ struct fw_card {
        int current_tlabel;
        u64 tlabel_mask;
        struct list_head transaction_list;
-       struct timer_list flush_timer;
        unsigned long reset_jiffies;
 
        unsigned long long guid;
@@ -288,6 +287,8 @@ struct fw_transaction {
        int tlabel;
        int timestamp;
        struct list_head link;
+       struct fw_card *card;
+       struct timer_list split_timeout_timer;
 
        struct fw_packet packet;