Skip to content

Commit a6176b7

Browse files
committed
Revert "firewire: core: shrink critical section of fw_card spinlock in bm_work"
This reverts commit 5823103. The bus manager work has the race condition against fw_destroy_nodes() called by fw_core_remove_card(). The acquition of spin lock of fw_card is left as is again. Link: https://lore.kernel.org/r/20250924131823.262136-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
1 parent 8ec6a8e commit a6176b7

1 file changed

Lines changed: 30 additions & 8 deletions

File tree

drivers/firewire/core-card.c

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ enum bm_contention_outcome {
299299
};
300300

301301
static enum bm_contention_outcome contend_for_bm(struct fw_card *card)
302+
__must_hold(&card->lock)
302303
{
303304
int generation = card->generation;
304305
int local_id = card->local_node->node_id;
@@ -311,8 +312,11 @@ static enum bm_contention_outcome contend_for_bm(struct fw_card *card)
311312
bool keep_this_irm = false;
312313
struct fw_node *irm_node;
313314
struct fw_device *irm_device;
315+
int irm_node_id;
314316
int rcode;
315317

318+
lockdep_assert_held(&card->lock);
319+
316320
if (!grace) {
317321
if (!is_next_generation(generation, card->bm_generation) || card->bm_abdicate)
318322
return BM_CONTENTION_OUTCOME_WITHIN_WINDOW;
@@ -338,10 +342,16 @@ static enum bm_contention_outcome contend_for_bm(struct fw_card *card)
338342
return BM_CONTENTION_OUTCOME_IRM_COMPLIES_1394_1995_ONLY;
339343
}
340344

341-
rcode = fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP, irm_node->node_id, generation,
345+
irm_node_id = irm_node->node_id;
346+
347+
spin_unlock_irq(&card->lock);
348+
349+
rcode = fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP, irm_node_id, generation,
342350
SCODE_100, CSR_REGISTER_BASE + CSR_BUS_MANAGER_ID, data,
343351
sizeof(data));
344352

353+
spin_lock_irq(&card->lock);
354+
345355
switch (rcode) {
346356
case RCODE_GENERATION:
347357
return BM_CONTENTION_OUTCOME_AT_NEW_GENERATION;
@@ -352,12 +362,10 @@ static enum bm_contention_outcome contend_for_bm(struct fw_card *card)
352362
int bm_id = be32_to_cpu(data[0]);
353363

354364
// Used by cdev layer for "struct fw_cdev_event_bus_reset".
355-
scoped_guard(spinlock, &card->lock) {
356-
if (bm_id != BUS_MANAGER_ID_NOT_REGISTERED)
357-
card->bm_node_id = 0xffc0 & bm_id;
358-
else
359-
card->bm_node_id = local_id;
360-
}
365+
if (bm_id != BUS_MANAGER_ID_NOT_REGISTERED)
366+
card->bm_node_id = 0xffc0 & bm_id;
367+
else
368+
card->bm_node_id = local_id;
361369

362370
if (bm_id != BUS_MANAGER_ID_NOT_REGISTERED)
363371
return BM_CONTENTION_OUTCOME_IRM_HOLDS_ANOTHER_NODE_AS_BM;
@@ -389,8 +397,12 @@ static void bm_work(struct work_struct *work)
389397
int expected_gap_count, generation;
390398
bool stand_for_root = false;
391399

392-
if (card->local_node == NULL)
400+
spin_lock_irq(&card->lock);
401+
402+
if (card->local_node == NULL) {
403+
spin_unlock_irq(&card->lock);
393404
return;
405+
}
394406

395407
generation = card->generation;
396408

@@ -405,6 +417,7 @@ static void bm_work(struct work_struct *work)
405417

406418
switch (result) {
407419
case BM_CONTENTION_OUTCOME_WITHIN_WINDOW:
420+
spin_unlock_irq(&card->lock);
408421
fw_schedule_bm_work(card, msecs_to_jiffies(125));
409422
return;
410423
case BM_CONTENTION_OUTCOME_IRM_HAS_LINK_OFF:
@@ -415,10 +428,12 @@ static void bm_work(struct work_struct *work)
415428
break;
416429
case BM_CONTENTION_OUTCOME_AT_NEW_GENERATION:
417430
// BM work has been rescheduled.
431+
spin_unlock_irq(&card->lock);
418432
return;
419433
case BM_CONTENTION_OUTCOME_LOCAL_PROBLEM_AT_TRANSACTION:
420434
// Let's try again later and hope that the local problem has gone away by
421435
// then.
436+
spin_unlock_irq(&card->lock);
422437
fw_schedule_bm_work(card, msecs_to_jiffies(125));
423438
return;
424439
case BM_CONTENTION_OUTCOME_IRM_IS_NOT_CAPABLE_FOR_IRM:
@@ -428,7 +443,9 @@ static void bm_work(struct work_struct *work)
428443
case BM_CONTENTION_OUTCOME_IRM_HOLDS_ANOTHER_NODE_AS_BM:
429444
if (local_id == irm_id) {
430445
// Only acts as IRM.
446+
spin_unlock_irq(&card->lock);
431447
allocate_broadcast_channel(card, generation);
448+
spin_lock_irq(&card->lock);
432449
}
433450
fallthrough;
434451
case BM_CONTENTION_OUTCOME_IRM_HOLDS_LOCAL_NODE_AS_BM:
@@ -469,6 +486,7 @@ static void bm_work(struct work_struct *work)
469486
if (!root_device_is_running) {
470487
// If we haven't probed this device yet, bail out now
471488
// and let's try again once that's done.
489+
spin_unlock_irq(&card->lock);
472490
return;
473491
} else if (!root_device->cmc) {
474492
// Current root has an active link layer and we
@@ -504,6 +522,8 @@ static void bm_work(struct work_struct *work)
504522
if (card->bm_retries++ < 5 && (card->gap_count != expected_gap_count || new_root_id != root_id)) {
505523
int card_gap_count = card->gap_count;
506524

525+
spin_unlock_irq(&card->lock);
526+
507527
fw_notice(card, "phy config: new root=%x, gap_count=%d\n",
508528
new_root_id, expected_gap_count);
509529
fw_send_phy_config(card, new_root_id, generation, expected_gap_count);
@@ -524,6 +544,8 @@ static void bm_work(struct work_struct *work)
524544
} else {
525545
struct fw_device *root_device = fw_node_get_device(root_node);
526546

547+
spin_unlock_irq(&card->lock);
548+
527549
if (root_device && root_device->cmc) {
528550
// Make sure that the cycle master sends cycle start packets.
529551
__be32 data = cpu_to_be32(CSR_STATE_BIT_CMSTR);

0 commit comments

Comments
 (0)