* [PATCH 1/3] firewire: core: refactor invoking transaction callback
2026-09-21 3:19 [PATCH 0/3] firewire: core: invoke transaction callbacks in process context Takashi Sakamoto
@ 2026-09-21 3:19 ` Takashi Sakamoto
2026-09-21 3:19 ` [PATCH 2/3] firewire: core: use workqueue to invoke transaction callback in some error cases Takashi Sakamoto
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2026-09-21 3:19 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/3] firewire: core: use workqueue to invoke transaction callback in some error cases
2026-09-21 3:19 [PATCH 0/3] firewire: core: invoke transaction callbacks in process context Takashi Sakamoto
2026-09-21 3:19 ` [PATCH 1/3] firewire: core: refactor invoking transaction callback Takashi Sakamoto
@ 2026-09-21 3:19 ` Takashi Sakamoto
2026-09-21 3:19 ` [PATCH 3/3] firewire: core: update kerneldoc for fw_send_request() and its variants Takashi Sakamoto
2026-09-23 0:24 ` [PATCH 0/3] firewire: core: invoke transaction callbacks in process context Takashi Sakamoto
3 siblings, 0 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2026-09-21 3:19 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 3/3] firewire: core: update kerneldoc for fw_send_request() and its variants
2026-09-21 3:19 [PATCH 0/3] firewire: core: invoke transaction callbacks in process context Takashi Sakamoto
2026-09-21 3:19 ` [PATCH 1/3] firewire: core: refactor invoking transaction callback Takashi Sakamoto
2026-09-21 3:19 ` [PATCH 2/3] firewire: core: use workqueue to invoke transaction callback in some error cases Takashi Sakamoto
@ 2026-09-21 3:19 ` Takashi Sakamoto
2026-09-23 0:24 ` [PATCH 0/3] firewire: core: invoke transaction callbacks in process context Takashi Sakamoto
3 siblings, 0 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2026-09-21 3:19 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread