* [PATCH 1/2] firewire: core: code refactoring to remove transaction entry
2025-11-01 10:21 [PATCH 0/2] firewire: core: code refactoring for transaction layer Takashi Sakamoto
@ 2025-11-01 10:21 ` Takashi Sakamoto
2025-11-01 10:21 ` [PATCH 2/2] firewire: core: code refactoring to find and pop " Takashi Sakamoto
2025-11-06 0:45 ` [PATCH 0/2] firewire: core: code refactoring for transaction layer Takashi Sakamoto
2 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2025-11-01 10:21 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
The list operation to remove transaction entry appears several times in
transaction implementation and can be replaced with a helper function.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-transaction.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index dd3656a0c1ff..8bd79c3f97f2 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -44,6 +44,13 @@ static int try_cancel_split_timeout(struct fw_transaction *t)
return 1;
}
+// card->transactions.lock must be acquired in advance.
+static void remove_transaction_entry(struct fw_card *card, struct fw_transaction *entry)
+{
+ list_del_init(&entry->link);
+ card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
+}
+
static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode,
u32 response_tstamp)
{
@@ -55,8 +62,7 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card
list_for_each_entry(iter, &card->transactions.list, link) {
if (iter == transaction) {
if (try_cancel_split_timeout(iter)) {
- list_del_init(&iter->link);
- card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
+ remove_transaction_entry(card, iter);
t = iter;
}
break;
@@ -122,8 +128,7 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
scoped_guard(spinlock_irqsave, &card->transactions.lock) {
if (list_empty(&t->link))
return;
- list_del(&t->link);
- card->transactions.tlabel_mask &= ~(1ULL << t->tlabel);
+ remove_transaction_entry(card, t);
}
if (!t->with_tstamp) {
@@ -1142,8 +1147,7 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
list_for_each_entry(iter, &card->transactions.list, link) {
if (iter->node_id == source && iter->tlabel == tlabel) {
if (try_cancel_split_timeout(iter)) {
- list_del_init(&iter->link);
- card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
+ remove_transaction_entry(card, iter);
t = iter;
}
break;
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] firewire: core: code refactoring to find and pop transaction entry
2025-11-01 10:21 [PATCH 0/2] firewire: core: code refactoring for transaction layer Takashi Sakamoto
2025-11-01 10:21 ` [PATCH 1/2] firewire: core: code refactoring to remove transaction entry Takashi Sakamoto
@ 2025-11-01 10:21 ` Takashi Sakamoto
2025-11-06 0:45 ` [PATCH 0/2] firewire: core: code refactoring for transaction layer Takashi Sakamoto
2 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2025-11-01 10:21 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
The list operation to find and pop transaction entry appears several
times in transaction implementation, and can be replaced with a helper
functional macro.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-transaction.c | 45 ++++++++++++++---------------
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 8bd79c3f97f2..e80791d6d46b 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -51,28 +51,34 @@ static void remove_transaction_entry(struct fw_card *card, struct fw_transaction
card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
}
+// card->transactions.lock must be acquired in advance.
+#define find_and_pop_transaction_entry(card, condition) \
+({ \
+ struct fw_transaction *iter, *t = NULL; \
+ list_for_each_entry(iter, &card->transactions.list, link) { \
+ if (condition) { \
+ t = iter; \
+ break; \
+ } \
+ } \
+ if (t && try_cancel_split_timeout(t)) \
+ remove_transaction_entry(card, t); \
+ t; \
+})
+
static int close_transaction(struct fw_transaction *transaction, struct fw_card *card, int rcode,
u32 response_tstamp)
{
- struct fw_transaction *t = NULL, *iter;
+ struct fw_transaction *t;
// 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_for_each_entry(iter, &card->transactions.list, link) {
- if (iter == transaction) {
- if (try_cancel_split_timeout(iter)) {
- remove_transaction_entry(card, iter);
- t = iter;
- }
- break;
- }
- }
+ t = find_and_pop_transaction_entry(card, iter == transaction);
+ if (!t)
+ return -ENOENT;
}
- if (!t)
- return -ENOENT;
-
if (!t->with_tstamp) {
t->callback.without_tstamp(card, rcode, NULL, 0, t->callback_data);
} else {
@@ -1102,7 +1108,7 @@ EXPORT_SYMBOL(fw_core_handle_request);
void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
{
- struct fw_transaction *t = NULL, *iter;
+ struct fw_transaction *t = NULL;
u32 *data;
size_t data_length;
int tcode, tlabel, source, rcode;
@@ -1144,15 +1150,8 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
// 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_for_each_entry(iter, &card->transactions.list, link) {
- if (iter->node_id == source && iter->tlabel == tlabel) {
- if (try_cancel_split_timeout(iter)) {
- remove_transaction_entry(card, iter);
- t = iter;
- }
- break;
- }
- }
+ t = find_and_pop_transaction_entry(card,
+ iter->node_id == source && iter->tlabel == tlabel);
}
trace_async_response_inbound((uintptr_t)t, card->index, p->generation, p->speed, p->ack,
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread