[PATCH 1/6] firewire: core: use spinlock_irqsave() appropriately for transaction list lock

[Takashi Sakamoto]

The transaction.lock in the fw_card structure protects the transaction
list from concurrent access. The list is mostly accessed from process
context, with two exceptions: split_transaction_timeout_callback()
acquires the lock in softIRQ context for the timer wheel, and
__fw_send_request() acquires it in the caller's context, which can
include hardIRQ context.

Use spinlock_irq() in process context and spinlock_irqsave() in the other
contexts. Update the relevant scoped_guard() invocations accordingly and
remove the unnecessary comments.

Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
---
 drivers/firewire/core-transaction.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index a0379fc3e60e..39715cf79ab9 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -69,9 +69,7 @@ void fw_cancel_pending_transactions(struct fw_card *card)
 	struct fw_transaction *t, *tmp;
 	LIST_HEAD(pending_list);
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
+	scoped_guard(spinlock_irq, &card->transactions.lock) {
 		list_for_each_entry_safe(t, tmp, &card->transactions.list, link) {
 			if (try_cancel_split_timeout(t))
 				list_move(&t->link, &pending_list);
@@ -121,10 +119,7 @@ int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transacti
 
 	// If the request packet has already been sent, we need to see if the transaction is still
 	// pending and remove it in that case (e.g. the split transaction).
-	//
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
+	scoped_guard(spinlock_irq, &card->transactions.lock) {
 		if (!find_and_pop_transaction_entry(card, iter == transaction))
 			return -ENOENT;
 	}
@@ -204,9 +199,7 @@ static void transmit_complete_callback(struct fw_packet *packet,
 			delta = card->split_timeout.jiffies;
 		}
 
-		// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-		// local destination never runs in any type of IRQ context.
-		scoped_guard(spinlock_irqsave, &card->transactions.lock)
+		scoped_guard(spinlock_irq, &card->transactions.lock)
 			start_split_transaction_timeout(t, delta);
 		return;
 	}
@@ -230,9 +223,7 @@ static void transmit_complete_callback(struct fw_packet *packet,
 		break;
 	}
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
+	scoped_guard(spinlock_irq, &card->transactions.lock) {
 		if (!find_and_pop_transaction_entry(card, iter == t))
 			return;
 	}
@@ -399,8 +390,6 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 	 * the list while holding the card spinlock.
 	 */
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->transactions.lock)
 		tlabel = allocate_tlabel(card);
 	if (tlabel < 0) {
@@ -423,16 +412,12 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 	timer_setup(&t->split_timeout_timer, split_transaction_timeout_callback, 0);
 	t->packet.callback = transmit_complete_callback;
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->lock) {
 		// The node_id field of fw_card can be updated when handling SelfIDComplete.
 		fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node_id,
 				generation, speed, offset, payload, length);
 	}
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->transactions.lock)
 		list_add_tail(&t->link, &card->transactions.list);
 
@@ -1176,9 +1161,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 		break;
 	}
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
-	// local destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
+	scoped_guard(spinlock_irq, &card->transactions.lock) {
 		t = find_and_pop_transaction_entry(card,
 				iter->node_id == source && iter->tlabel == tlabel);
 	}
-- 
2.53.0