[PATCH 0/3] firewire: core: simplify pending transaction cancellation on card removal

[PATCH 0/3] firewire: core: simplify pending transaction cancellation on card removal

[Takashi Sakamoto]

Hi,

This series simplifies the handling of pending transactions by using
workqueue when a card is removed. It also improves the checking of
preconditions for scheduling error callbacks by moving the warning check
to the scheduling helper and using compiler-based context analysis to
verify lock state.


Takashi Sakamoto (3):
  firewire: core: use workqueue to cancel all pending transactions on
    card removal
  firewire: core: check precondition when scheduling error callbacks
  firewire: core: use context analysis to check lock state when
    scheduling error callbacks

 drivers/firewire/core-card.c        |  4 +-
 drivers/firewire/core-transaction.c | 76 ++++++++++++++---------------
 2 files changed, 40 insertions(+), 40 deletions(-)


base-commit: 7b00b66bdb92a0028ecb7fe343d4517dc1e95017
-- 
2.53.0

[PATCH 1/3] firewire: core: use workqueue to cancel all pending transactions on card removal

[Takashi Sakamoto]

When a card is being removed, all pending transactions are canceled by
calling fw_cancel_pending_transactions(). This function moves the entries
for unfinished transactions to a local list so that their callbacks can
be invoked without holding the transaction lock. However, this complicates
the management of pending transactions.

Use workqueue to invoke error callbacks for all pending transactions.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-card.c        |  4 ++-
 drivers/firewire/core-transaction.c | 38 ++++++++++++-----------------
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 83c29a487d86..a43fc3a278f2 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -811,7 +811,9 @@ void fw_core_remove_card(struct fw_card *card)
 	drain_workqueue(card->isoc_wq);
 	drain_workqueue(card->async_wq);
 	card->driver->disable(card);
-	fw_cancel_pending_transactions(card);
+
+	scoped_guard(spinlock_irqsave, &card->transactions.lock)
+		fw_cancel_pending_transactions(card);
 
 	scoped_guard(spinlock_irqsave, &card->lock)
 		fw_destroy_nodes(card);
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 96c86a5b81f9..31cc440d4218 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -69,28 +69,6 @@ static void invoke_callback(struct fw_transaction *t, int rcode, u32 response_ts
 	}
 }
 
-// Must be called without holding card->transactions.lock.
-void fw_cancel_pending_transactions(struct fw_card *card)
-__must_not_hold(&card->transactions.lock)
-{
-	struct fw_transaction *t, *tmp;
-	LIST_HEAD(pending_list);
-
-	lockdep_assert_not_held(&card->transactions.lock);
-
-	scoped_guard(spinlock_irqsave, &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);
-		}
-	}
-
-	list_for_each_entry_safe(t, tmp, &pending_list, link) {
-		list_del(&t->link);
-		invoke_callback(t, RCODE_CANCELLED, 0, NULL, 0);
-	}
-}
-
 // card->transactions.lock must be acquired in advance.
 #define find_and_pop_transaction_entry(card, condition)			\
 ({									\
@@ -114,6 +92,22 @@ static void schedule_error_callback(struct fw_transaction *t, int rcode, u32 res
 	queue_work(t->card->async_wq, &t->error_work);
 }
 
+// Must be called without holding card->transactions.lock.
+void fw_cancel_pending_transactions(struct fw_card *card)
+__must_hold(&card->transactions.lock)
+{
+	struct fw_transaction *t, *tmp;
+
+	lockdep_assert_held(&card->transactions.lock);
+
+	list_for_each_entry_safe(t, tmp, &card->transactions.list, link) {
+		if (try_cancel_split_timeout(t)) {
+			remove_transaction_entry(card, t);
+			schedule_error_callback(t, RCODE_CANCELLED, 0);
+		}
+	}
+}
+
 /**
  * fw_cancel_transaction: cancel transaction.
  * @card:		interface where the transaction has been requested
-- 
2.53.0

[PATCH 2/3] firewire: core: check precondition when scheduling error callbacks

[Takashi Sakamoto]

Current implementation requires that the transaction not be on the
pending list before its error callback is scheduled. A WARN_ON() in the
callback function detects violations of this precondition, but the check
belongs in the scheduling helper.

Move the WARN_ON() to schedule_error_callback().

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 31cc440d4218..33f3f272e99b 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -84,8 +84,14 @@ static void invoke_callback(struct fw_transaction *t, int rcode, u32 response_ts
 	__t;								\
 })
 
+// Ensure that the transaction is not on the pending list when scheduling this work. Cancelling
+// the work after scheduling makes no sense, as the initiator must be notified of the transaction
+// result.
 static void schedule_error_callback(struct fw_transaction *t, int rcode, u32 response_timestamp)
 {
+	// To detect programming mistakes.
+	WARN_ON(!list_empty(&t->link));
+
 	t->rcode = rcode;
 	t->response_timestamp = response_timestamp;
 
@@ -153,16 +159,10 @@ __must_not_hold(&card->transactions.lock)
 }
 EXPORT_SYMBOL(fw_cancel_transaction);
 
-// Ensure that the transaction is not on the pending list when scheduling this work. Cancelling
-// the work after scheduling makes no sense, as the initiator must be notified of the transaction
-// result.
 static void error_callback_work(struct work_struct *work)
 {
 	struct fw_transaction *t = from_work(t, work, error_work);
 
-	// To detect programming mistakes.
-	WARN_ON(!list_empty(&t->link));
-
 	invoke_callback(t, t->rcode, t->response_timestamp, NULL, 0);
 }
 
-- 
2.53.0

[PATCH 3/3] firewire: core: use context analysis to check lock state when scheduling error callbacks

[Takashi Sakamoto]

The precondition for schedule_error_callback() involves accessing the
pending transaction list. This access should be protected by the
transaction lock.

Use compiler-based context analysis to verify that the lock is held when
accessing the list.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 33f3f272e99b..b633d343c8b9 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -87,8 +87,12 @@ static void invoke_callback(struct fw_transaction *t, int rcode, u32 response_ts
 // Ensure that the transaction is not on the pending list when scheduling this work. Cancelling
 // the work after scheduling makes no sense, as the initiator must be notified of the transaction
 // result.
-static void schedule_error_callback(struct fw_transaction *t, int rcode, u32 response_timestamp)
+static void schedule_error_callback(struct fw_card *card, struct fw_transaction *t, int rcode,
+				    u32 response_timestamp)
+__must_hold(&card->transactions.lock)
 {
+	lockdep_assert_held(&card->transactions.lock);
+
 	// To detect programming mistakes.
 	WARN_ON(!list_empty(&t->link));
 
@@ -109,7 +113,7 @@ __must_hold(&card->transactions.lock)
 	list_for_each_entry_safe(t, tmp, &card->transactions.list, link) {
 		if (try_cancel_split_timeout(t)) {
 			remove_transaction_entry(card, t);
-			schedule_error_callback(t, RCODE_CANCELLED, 0);
+			schedule_error_callback(card, t, RCODE_CANCELLED, 0);
 		}
 	}
 }
@@ -140,21 +144,21 @@ __must_not_hold(&card->transactions.lock)
 	if (smp_load_acquire(&card->driver)->cancel_packet(card, &transaction->packet) == 0)
 		return 0;
 
+	u32 curr_cycle_time = 0;
+
+	// Timestamping on behalf of hardware.
+	(void)fw_card_read_cycle_time(card, &curr_cycle_time);
+
+	u32 tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time);
+
 	// 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).
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
 		if (!find_and_pop_transaction_entry(card, iter == transaction))
 			return -ENOENT;
+		schedule_error_callback(card, transaction, RCODE_CANCELLED, tstamp);
 	}
 
-	u32 curr_cycle_time = 0;
-
-	// Timestamping on behalf of hardware.
-	(void)fw_card_read_cycle_time(card, &curr_cycle_time);
-
-	schedule_error_callback(transaction, RCODE_CANCELLED,
-				cycle_time_to_ohci_tstamp(curr_cycle_time));
-
 	return 0;
 }
 EXPORT_SYMBOL(fw_cancel_transaction);
@@ -178,9 +182,9 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
 		if (list_empty(&t->link))
 			return;
 		remove_transaction_entry(card, t);
+		schedule_error_callback(card, t, RCODE_CANCELLED, t->split_timeout_cycle);
 	}
 
-	schedule_error_callback(t, RCODE_CANCELLED, t->split_timeout_cycle);
 }
 
 // card->transactions.lock should be acquired in advance for the linked list.
@@ -431,7 +435,7 @@ __must_not_hold(&card->transactions.lock)
 		tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time);
 
 		t->packet.timestamp = tstamp;
-		schedule_error_callback(t, RCODE_SEND_ERROR, tstamp);
+		context_unsafe(schedule_error_callback(card, t, RCODE_SEND_ERROR, tstamp));
 
 		return;
 	}
-- 
2.53.0

Re: [PATCH 0/3] firewire: core: simplify pending transaction cancellation on card removal

[Takashi Sakamoto]

On Sun, Oct 18, 2026 at 03:30:53PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> This series simplifies the handling of pending transactions by using
> workqueue when a card is removed. It also improves the checking of
> preconditions for scheduling error callbacks by moving the warning check
> to the scheduling helper and using compiler-based context analysis to
> verify lock state.
> 
> 
> Takashi Sakamoto (3):
>   firewire: core: use workqueue to cancel all pending transactions on
>     card removal
>   firewire: core: check precondition when scheduling error callbacks
>   firewire: core: use context analysis to check lock state when
>     scheduling error callbacks
> 
>  drivers/firewire/core-card.c        |  4 +-
>  drivers/firewire/core-transaction.c | 76 ++++++++++++++---------------
>  2 files changed, 40 insertions(+), 40 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto