[PATCH 0/3] firewire: core: invoke transaction callbacks in process context

[PATCH 0/3] firewire: core: invoke transaction callbacks in process context

[Takashi Sakamoto]

Hi,

Currently, some error paths invoke the transaction callback in the
caller's context, while a split transaction timeout invokes it from
softIRQ context. This makes the execution context of the callback
inconsistent.

This series ensures that transaction callbacks are always invoked in
process context by using a workqueue for the error paths and split
transaction timeout.


Takashi Sakamoto (3):
  firewire: core: refactor invoking transaction callback
  firewire: core: use workqueue to invoke transaction callback in some
    error cases
  firewire: core: update kerneldoc for fw_send_request() and its
    variants

 drivers/firewire/core-transaction.c | 91 +++++++++++++++--------------
 include/linux/firewire.h            | 15 +++--
 2 files changed, 56 insertions(+), 50 deletions(-)


base-commit: bbc5886fb1ee5d29438c81a770ea4bb190757b3f
-- 
2.53.0

[PATCH 1/3] firewire: core: refactor invoking transaction callback

[Takashi Sakamoto]

Two types of transaction callbacks are defined. There are many places
where the transaction callback is invoked with conditional branches.

Add a helper function to simplify invoking the callback.

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

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index a2a8d755ad0a..0cad55763a3e 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -52,6 +52,17 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction
 	card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
 }
 
+static void invoke_callback(struct fw_transaction *t, int rcode, u32 response_tstamp, void *data,
+			    size_t data_length)
+{
+	if (!t->with_tstamp) {
+		t->callback.without_tstamp(t->card, rcode, data, data_length, t->callback_data);
+	} else {
+		t->callback.with_tstamp(t->card, rcode, t->packet.timestamp, response_tstamp,
+					data, data_length, t->callback_data);
+	}
+}
+
 // Must be called without holding card->transactions.lock.
 void fw_cancel_pending_transactions(struct fw_card *card)
 {
@@ -69,14 +80,7 @@ void fw_cancel_pending_transactions(struct fw_card *card)
 
 	list_for_each_entry_safe(t, tmp, &pending_list, link) {
 		list_del(&t->link);
-
-		if (!t->with_tstamp) {
-			t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0,
-						   t->callback_data);
-		} else {
-			t->callback.with_tstamp(card, RCODE_CANCELLED, t->packet.timestamp, 0,
-						NULL, 0, t->callback_data);
-		}
+		invoke_callback(t, RCODE_CANCELLED, 0, NULL, 0);
 	}
 }
 
@@ -108,12 +112,7 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card
 			return -ENOENT;
 	}
 
-	if (!t->with_tstamp) {
-		t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data);
-	} else {
-		t->callback.with_tstamp(card, rcode, t->packet.timestamp, response_tstamp, NULL, 0,
-					t->callback_data);
-	}
+	invoke_callback(t, rcode, response_tstamp, NULL, 0);
 
 	return 0;
 }
@@ -166,12 +165,7 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
 		remove_transaction_entry(card, t);
 	}
 
-	if (!t->with_tstamp) {
-		t->callback.without_tstamp(card, RCODE_CANCELLED, NULL, 0, t->callback_data);
-	} else {
-		t->callback.with_tstamp(card, RCODE_CANCELLED, t->packet.timestamp,
-					t->split_timeout_cycle, NULL, 0, t->callback_data);
-	}
+	invoke_callback(t, RCODE_CANCELLED, t->split_timeout_cycle, NULL, 0);
 }
 
 // card->transactions.lock should be acquired in advance for the linked list.
@@ -385,6 +379,11 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 {
 	int tlabel;
 
+	t->card = card;
+	t->callback = callback;
+	t->with_tstamp = with_tstamp;
+	t->callback_data = callback_data;
+
 	/*
 	 * Allocate tlabel from the bitmap and put the transaction on
 	 * the list while holding the card spinlock.
@@ -395,30 +394,23 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 	scoped_guard(spinlock_irqsave, &card->transactions.lock)
 		tlabel = allocate_tlabel(card);
 	if (tlabel < 0) {
-		if (!with_tstamp) {
-			callback.without_tstamp(card, RCODE_SEND_ERROR, NULL, 0, callback_data);
-		} else {
-			// Timestamping on behalf of hardware.
-			u32 curr_cycle_time = 0;
-			u32 tstamp;
+		// Timestamping on behalf of hardware.
+		u32 curr_cycle_time = 0;
+		u32 tstamp;
 
-			(void)fw_card_read_cycle_time(card, &curr_cycle_time);
-			tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time);
+		(void)fw_card_read_cycle_time(card, &curr_cycle_time);
+		tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time);
+
+		t->packet.timestamp = tstamp;
+		invoke_callback(t, RCODE_SEND_ERROR, tstamp, NULL, 0);
 
-			callback.with_tstamp(card, RCODE_SEND_ERROR, tstamp, tstamp, NULL, 0,
-					     callback_data);
-		}
 		return;
 	}
 
 	t->node_id = destination_id;
 	t->tlabel = tlabel;
-	t->card = card;
 	t->is_split_transaction = false;
 	timer_setup(&t->split_timeout_timer, split_transaction_timeout_callback, 0);
-	t->callback = callback;
-	t->with_tstamp = with_tstamp;
-	t->callback_data = callback_data;
 	t->packet.callback = transmit_complete_callback;
 
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
@@ -1198,12 +1190,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	 */
 	card->driver->cancel_packet(card, &t->packet);
 
-	if (!t->with_tstamp) {
-		t->callback.without_tstamp(card, rcode, data, data_length, t->callback_data);
-	} else {
-		t->callback.with_tstamp(card, rcode, t->packet.timestamp, p->timestamp, data,
-					data_length, t->callback_data);
-	}
+	invoke_callback(t, rcode, p->timestamp, data, data_length);
 }
 EXPORT_SYMBOL(fw_core_handle_response);
 
-- 
2.53.0

[PATCH 2/3] firewire: core: use workqueue to invoke transaction callback in some error cases

[Takashi Sakamoto]

Some error paths of __fw_send_request() invoke the transaction callback
in the caller's context. Additionally, when a split transaction times
out, the callback is invoked in softIRQ context by the timer wheel.

These are the only cases where the callback is not guaranteed to be
invoked in process context.

Use a workqueue to invoke the callback in these cases. This may introduce
additional delay when a split transaction times out, but the default
timeout is 2 seconds, so the additional delay should be negligible.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 22 +++++++++++++++++++---
 include/linux/firewire.h            |  5 +++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 0cad55763a3e..e59b347ff248 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -40,7 +40,7 @@
 static int try_cancel_split_timeout(struct fw_transaction *t)
 {
 	if (t->is_split_transaction)
-		return timer_delete(&t->split_timeout_timer);
+		return timer_delete(&t->split_timeout_timer) || disable_work(&t->error_work);
 	else
 		return 1;
 }
@@ -154,6 +154,21 @@ int fw_cancel_transaction(struct fw_card *card,
 }
 EXPORT_SYMBOL(fw_cancel_transaction);
 
+static void error_callback_work(struct work_struct *work)
+{
+	struct fw_transaction *t = from_work(t, work, error_work);
+
+	invoke_callback(t, t->rcode, t->response_timestamp, NULL, 0);
+}
+
+static void schedule_error_callback(struct fw_transaction *t, int rcode, u32 response_timestamp)
+{
+	t->rcode = rcode;
+	t->response_timestamp = response_timestamp;
+
+	queue_work(t->card->async_wq, &t->error_work);
+}
+
 static void split_transaction_timeout_callback(struct timer_list *timer)
 {
 	struct fw_transaction *t = timer_container_of(t, timer, split_timeout_timer);
@@ -165,7 +180,7 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
 		remove_transaction_entry(card, t);
 	}
 
-	invoke_callback(t, RCODE_CANCELLED, t->split_timeout_cycle, NULL, 0);
+	schedule_error_callback(t, RCODE_CANCELLED, t->split_timeout_cycle);
 }
 
 // card->transactions.lock should be acquired in advance for the linked list.
@@ -383,6 +398,7 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 	t->callback = callback;
 	t->with_tstamp = with_tstamp;
 	t->callback_data = callback_data;
+	INIT_WORK(&t->error_work, error_callback_work);
 
 	/*
 	 * Allocate tlabel from the bitmap and put the transaction on
@@ -402,7 +418,7 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 		tstamp = cycle_time_to_ohci_tstamp(curr_cycle_time);
 
 		t->packet.timestamp = tstamp;
-		invoke_callback(t, RCODE_SEND_ERROR, tstamp, NULL, 0);
+		schedule_error_callback(t, RCODE_SEND_ERROR, tstamp);
 
 		return;
 	}
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 2b065f03565d..a3a3a9ec1b21 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -362,6 +362,11 @@ struct fw_transaction {
 	union fw_transaction_callback callback;
 	bool with_tstamp;
 	void *callback_data;
+
+	// For some error cases.
+	struct work_struct error_work;
+	int rcode;
+	u32 response_timestamp;
 };
 
 struct fw_address_handler {
-- 
2.53.0

[PATCH 3/3] firewire: core: update kerneldoc for fw_send_request() and its variants

[Takashi Sakamoto]

Some error paths invoke the transaction callback in process context using
a workqueue. All transaction callbacks are now invoked in process context.

Update kerneldoc to reflect this behavior.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c |  4 ++--
 include/linux/firewire.h            | 10 ++++------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index e59b347ff248..8ef04d84011b 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -374,8 +374,8 @@ __must_hold(&card->transactions.lock)
  *
  * In case of request types without payload, @data is NULL and @length is 0.
  *
- * After the transaction is completed successfully or unsuccessfully, the
- * @callback will be called.  Among its parameters is the response code which
+ * After the transaction is completed successfully or unsuccessfully, the @callback will be called
+ * in process context.  Among its parameters is the response code which
  * is either one of the rcodes per IEEE 1394 or, in case of internal errors,
  * the firewire-core specific %RCODE_SEND_ERROR.  The other firewire-core
  * specific rcodes (%RCODE_CANCELLED, %RCODE_BUSY, %RCODE_GENERATION,
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index a3a3a9ec1b21..eebadebe5936 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -419,9 +419,8 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
  * A variation of __fw_send_request() to generate callback for response subaction without time
  * stamp.
  *
- * The callback is invoked in the workqueue context in most cases. However, if an error is detected
- * before queueing or the destination address refers to the local node, it is invoked in the
- * current context instead.
+ * After the transaction is completed successfully or unsuccessfully, the @callback will be called
+ * in process context.
  */
 static inline void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode,
 				   int destination_id, int generation, int speed,
@@ -452,9 +451,8 @@ static inline void fw_send_request(struct fw_card *card, struct fw_transaction *
  *
  * A variation of __fw_send_request() to generate callback for response subaction with time stamp.
  *
- * The callback is invoked in the workqueue context in most cases. However, if an error is detected
- * before queueing or the destination address refers to the local node, it is invoked in the current
- * context instead.
+ * After the transaction is completed successfully or unsuccessfully, the @callback will be called
+ * in process context.
  */
 static inline void fw_send_request_with_tstamp(struct fw_card *card, struct fw_transaction *t,
 	int tcode, int destination_id, int generation, int speed, unsigned long long offset,
-- 
2.53.0

Re: [PATCH 0/3] firewire: core: invoke transaction callbacks in process context

[Takashi Sakamoto]

On Mon, Sep 21, 2026 at 12:19:28PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> Currently, some error paths invoke the transaction callback in the
> caller's context, while a split transaction timeout invokes it from
> softIRQ context. This makes the execution context of the callback
> inconsistent.
> 
> This series ensures that transaction callbacks are always invoked in
> process context by using a workqueue for the error paths and split
> transaction timeout.
> 
> 
> Takashi Sakamoto (3):
>   firewire: core: refactor invoking transaction callback
>   firewire: core: use workqueue to invoke transaction callback in some
>     error cases
>   firewire: core: update kerneldoc for fw_send_request() and its
>     variants
> 
>  drivers/firewire/core-transaction.c | 91 +++++++++++++++--------------
>  include/linux/firewire.h            | 15 +++--
>  2 files changed, 56 insertions(+), 50 deletions(-)

Applied to for-next.


Regards

Takashi Sakamoto