From 1d2f5118baa804592de725f445bffb907e6efaeb Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 7 Oct 2026 18:32:32 +0900 Subject: [PATCH] firewire: ohci: schedule callback on workqueue when cancelling AT request packet The current implementation invokes the packet callback in the context where the packet cancellation is requested. This is inconvenient because the context can affect the packet callback. Invoke the packet callback from a workqueue instead. Use the workqueue for local AT packets. Link: https://lore.kernel.org/r/20261007093235.175742-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-transaction.c | 8 ++---- drivers/firewire/ohci.c | 43 ++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 7a3bf8133ffc..bf39f82d9fc4 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -112,12 +112,8 @@ int fw_cancel_transaction(struct fw_card *card, { u32 tstamp; - /* - * Cancel the packet transmission if it's still queued. That - * will call the packet transmission callback which cancels - * the transaction. - */ - + // Cancel the packet transmission if it's still queued. That will schedule the packet + // transmission callback which cancels the transaction. if (card->driver->cancel_packet(card, &transaction->packet) == 0) return 0; diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index df53efec0b0a..896d0cb9a895 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1669,6 +1669,9 @@ static void at_context_transmit(struct at_context *ctx, struct fw_packet *packet struct fw_ohci *ohci = ctx->context.ohci; bool use_work = true; + // These members are used to cancel the packet. + packet->driver_data = NULL; + scoped_guard(spinlock_irqsave, &ohci->lock) { if (!destination_is_local(packet, ohci)) { if (!at_context_queue_packet(ctx, packet)) @@ -2652,7 +2655,7 @@ static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet) { struct fw_ohci *ohci = fw_ohci(card); struct at_context *ctx = &ohci->at_request_ctx; - struct driver_data *driver_data = packet->driver_data; + struct at_local *local = &ohci->at_request_local; int ret = -ENOENT; // Avoid dead lock due to programming mistake. @@ -2660,24 +2663,38 @@ static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet) return 0; disable_work_sync(&ctx->work); - if (packet->ack != 0) - goto out; + bool use_local_work = false; - if (packet->payload_mapped) - dma_unmap_single(ohci->card.device, packet->payload_bus, - packet->payload_length, DMA_TO_DEVICE); + if (!packet->ack) { + // NOTE: This function relies on this value, thus it is not thread-safe. + struct driver_data *driver_data = packet->driver_data; - driver_data->packet = NULL; - packet->ack = RCODE_CANCELLED; + if (driver_data) { + if (packet->payload_mapped) { + dma_unmap_single(ohci->card.device, packet->payload_bus, + packet->payload_length, DMA_TO_DEVICE); + } - // Timestamping on behalf of the hardware. - packet->timestamp = cycle_time_to_ohci_tstamp(get_cycle_time(ohci)); + // Invalidate the packet queued in AT request context. + driver_data->packet = NULL; + + scoped_guard(spinlock_irqsave, &local->lock) + list_add_tail(&packet->link_for_local, &local->list); + use_local_work = true; + } + } - packet->callback(packet, &ohci->card, packet->ack); - ret = 0; - out: enable_and_queue_work(card->async_wq, &ctx->work); + if (use_local_work) { + // Timestamping on behalf of the hardware. + packet->timestamp = cycle_time_to_ohci_tstamp(get_cycle_time(ohci)); + packet->ack = RCODE_CANCELLED; + queue_work(card->async_wq, &local->work); + + ret = 0; + } + return ret; }