Gh0st
/
linux
mirror of https://github.com/torvalds/linux.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

firewire: core: use spin lock specific to transaction 

The list of instance for asynchronous transaction to wait for response
subaction is maintained as a member of fw_card structure. The card-wide
spinlock is used at present for any operation over the list, however it
is not necessarily suited for the purpose.

This commit adds and uses the spin lock specific to maintain the list.

Link: https://lore.kernel.org/r/20250915234747.915922-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
This commit is contained in:
Takashi Sakamoto 2025-09-16 08:47:45 +09:00
parent 7d138cb269
commit 420bd7068c
3 changed files with 51 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
drivers/firewire/core-card.c
View File

@ -544,8 +544,12 @@ void fw_card_initialize(struct fw_card *card,
card->index = atomic_inc_return(&index); card->index = atomic_inc_return(&index);
card->driver = driver; card->driver = driver;
card->device = device; card->device = device;
card->current_tlabel = 0;
card->tlabel_mask = 0; card->transactions.current_tlabel = 0;
card->transactions.tlabel_mask = 0;
INIT_LIST_HEAD(&card->transactions.list);
spin_lock_init(&card->transactions.lock);
card->split_timeout_hi = DEFAULT_SPLIT_TIMEOUT / 8000; card->split_timeout_hi = DEFAULT_SPLIT_TIMEOUT / 8000;
card->split_timeout_lo = (DEFAULT_SPLIT_TIMEOUT % 8000) << 19; card->split_timeout_lo = (DEFAULT_SPLIT_TIMEOUT % 8000) << 19;
card->split_timeout_cycles = DEFAULT_SPLIT_TIMEOUT; card->split_timeout_cycles = DEFAULT_SPLIT_TIMEOUT;
@ -555,7 +559,7 @@ void fw_card_initialize(struct fw_card *card,
kref_init(&card->kref); kref_init(&card->kref);
init_completion(&card->done); init_completion(&card->done);
INIT_LIST_HEAD(&card->transaction_list);
spin_lock_init(&card->lock); spin_lock_init(&card->lock);
card->local_node = NULL; card->local_node = NULL;
@ -772,7 +776,7 @@ void fw_core_remove_card(struct fw_card *card)
destroy_workqueue(card->isoc_wq); destroy_workqueue(card->isoc_wq);
destroy_workqueue(card->async_wq); destroy_workqueue(card->async_wq);
WARN_ON(!list_empty(&card->transaction_list)); WARN_ON(!list_empty(&card->transactions.list));
} }
EXPORT_SYMBOL(fw_core_remove_card); EXPORT_SYMBOL(fw_core_remove_card);

59
drivers/firewire/core-transaction.c
View File

@ -49,12 +49,14 @@ static int close_transaction(struct fw_transaction *transaction, struct fw_card
{ {
struct fw_transaction *t = NULL, *iter; struct fw_transaction *t = NULL, *iter;
scoped_guard(spinlock_irqsave, &card->lock) { // NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
list_for_each_entry(iter, &card->transaction_list, link) { // 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 (iter == transaction) {
if (try_cancel_split_timeout(iter)) { if (try_cancel_split_timeout(iter)) {
list_del_init(&iter->link); list_del_init(&iter->link);
card->tlabel_mask &= ~(1ULL << iter->tlabel); card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
t = iter; t = iter;
} }
break; break;
@ -117,11 +119,11 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
struct fw_transaction *t = timer_container_of(t, timer, split_timeout_timer); struct fw_transaction *t = timer_container_of(t, timer, split_timeout_timer);
struct fw_card *card = t->card; struct fw_card *card = t->card;
scoped_guard(spinlock_irqsave, &card->lock) { scoped_guard(spinlock_irqsave, &card->transactions.lock) {
if (list_empty(&t->link)) if (list_empty(&t->link))
return; return;
list_del(&t->link); list_del(&t->link);
card->tlabel_mask &= ~(1ULL << t->tlabel); card->transactions.tlabel_mask &= ~(1ULL << t->tlabel);
} }
if (!t->with_tstamp) { if (!t->with_tstamp) {
@ -259,18 +261,21 @@ static void fw_fill_request(struct fw_packet *packet, int tcode, int tlabel,
} }
static int allocate_tlabel(struct fw_card *card) static int allocate_tlabel(struct fw_card *card)
__must_hold(&card->transactions_lock)
{ {
int tlabel; int tlabel;
tlabel = card->current_tlabel; lockdep_assert_held(&card->transactions.lock);
while (card->tlabel_mask & (1ULL << tlabel)) {
tlabel = card->transactions.current_tlabel;
while (card->transactions.tlabel_mask & (1ULL << tlabel)) {
tlabel = (tlabel + 1) & 0x3f; tlabel = (tlabel + 1) & 0x3f;
if (tlabel == card->current_tlabel) if (tlabel == card->transactions.current_tlabel)
return -EBUSY; return -EBUSY;
} }
card->current_tlabel = (tlabel + 1) & 0x3f; card->transactions.current_tlabel = (tlabel + 1) & 0x3f;
card->tlabel_mask |= 1ULL << tlabel; card->transactions.tlabel_mask |= 1ULL << tlabel;
return tlabel; return tlabel;
} }
@ -331,7 +336,6 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
void *payload, size_t length, union fw_transaction_callback callback, void *payload, size_t length, union fw_transaction_callback callback,
bool with_tstamp, void *callback_data) bool with_tstamp, void *callback_data)
{ {
unsigned long flags;
int tlabel; int tlabel;
/* /*
@ -339,11 +343,11 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
* the list while holding the card spinlock. * the list while holding the card spinlock.
*/ */
spin_lock_irqsave(&card->lock, flags); // 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.
tlabel = allocate_tlabel(card); scoped_guard(spinlock_irqsave, &card->transactions.lock)
tlabel = allocate_tlabel(card);
if (tlabel < 0) { if (tlabel < 0) {
spin_unlock_irqrestore(&card->lock, flags);
if (!with_tstamp) { if (!with_tstamp) {
callback.without_tstamp(card, RCODE_SEND_ERROR, NULL, 0, callback_data); callback.without_tstamp(card, RCODE_SEND_ERROR, NULL, 0, callback_data);
} else { } else {
@ -368,15 +372,22 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
t->callback = callback; t->callback = callback;
t->with_tstamp = with_tstamp; t->with_tstamp = with_tstamp;
t->callback_data = callback_data; t->callback_data = callback_data;
fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node_id, generation,
speed, offset, payload, length);
t->packet.callback = transmit_complete_callback; t->packet.callback = transmit_complete_callback;
list_add_tail(&t->link, &card->transaction_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->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);
}
spin_unlock_irqrestore(&card->lock, flags); // 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);
// Safe with no lock, since the index field of fw_card is immutable once assigned.
trace_async_request_outbound_initiate((uintptr_t)t, card->index, generation, speed, trace_async_request_outbound_initiate((uintptr_t)t, card->index, generation, speed,
t->packet.header, payload, t->packet.header, payload,
tcode_is_read_request(tcode) ? 0 : length / 4); tcode_is_read_request(tcode) ? 0 : length / 4);
@ -1111,12 +1122,14 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
break; break;
} }
scoped_guard(spinlock_irqsave, &card->lock) { // NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
list_for_each_entry(iter, &card->transaction_list, link) { // 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 (iter->node_id == source && iter->tlabel == tlabel) {
if (try_cancel_split_timeout(iter)) { if (try_cancel_split_timeout(iter)) {
list_del_init(&iter->link); list_del_init(&iter->link);
card->tlabel_mask &= ~(1ULL << iter->tlabel); card->transactions.tlabel_mask &= ~(1ULL << iter->tlabel);
t = iter; t = iter;
} }
break; break;

10
include/linux/firewire.h
View File

@ -88,11 +88,15 @@ struct fw_card {
int node_id; int node_id;
int generation; int generation;
int current_tlabel;
u64 tlabel_mask;
struct list_head transaction_list;
u64 reset_jiffies; u64 reset_jiffies;
struct {
int current_tlabel;
u64 tlabel_mask;
struct list_head list;
spinlock_t lock;
} transactions;
u32 split_timeout_hi; u32 split_timeout_hi;
u32 split_timeout_lo; u32 split_timeout_lo;
unsigned int split_timeout_cycles; unsigned int split_timeout_cycles;