From 6ffa9bd6ebce0626e62358dda59effe5758ebfc5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 12 Sep 2024 22:30:35 +0900 Subject: [PATCH] Revert "firewire: core: move workqueue handler from 1394 OHCI driver to core function" This reverts commit 767bfb9ef27ebf760290d9f8bc303828b018c312. It appears that the call of ohci_flush_iso_completions() in the work item scheduled by hardIRQ of 1394 OHCI for any isochronous context changes the timing to queue events in the view of user space application. Link: https://lore.kernel.org/r/20240912133038.238786-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-iso.c | 26 ++++++++++++--------- drivers/firewire/core.h | 5 +++++ drivers/firewire/ohci.c | 45 +++++++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 9f41c78878ad..f2394f3ed194 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -131,13 +131,6 @@ size_t fw_iso_buffer_lookup(struct fw_iso_buffer *buffer, dma_addr_t completed) return 0; } -static void flush_completions_work(struct work_struct *work) -{ - struct fw_iso_context *ctx = container_of(work, struct fw_iso_context, work); - - fw_iso_context_flush_completions(ctx); -} - struct fw_iso_context *fw_iso_context_create(struct fw_card *card, int type, int channel, int speed, size_t header_size, fw_iso_callback_t callback, void *callback_data) @@ -156,7 +149,6 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card, ctx->header_size = header_size; ctx->callback.sc = callback; ctx->callback_data = callback_data; - INIT_WORK(&ctx->work, flush_completions_work); trace_isoc_outbound_allocate(ctx, channel, speed); trace_isoc_inbound_single_allocate(ctx, channel, header_size); @@ -226,15 +218,29 @@ EXPORT_SYMBOL(fw_iso_context_queue_flush); * to process the context asynchronously, fw_iso_context_schedule_flush_completions() is available * instead. * - * Context: Process context. + * Context: Process context. May sleep due to disable_work_sync(). */ int fw_iso_context_flush_completions(struct fw_iso_context *ctx) { + int err; + trace_isoc_outbound_flush_completions(ctx); trace_isoc_inbound_single_flush_completions(ctx); trace_isoc_inbound_multiple_flush_completions(ctx); - return ctx->card->driver->flush_iso_completions(ctx); + might_sleep(); + + // Avoid dead lock due to programming mistake. + if (WARN_ON_ONCE(current_work() == &ctx->work)) + return 0; + + disable_work_sync(&ctx->work); + + err = ctx->card->driver->flush_iso_completions(ctx); + + enable_work(&ctx->work); + + return err; } EXPORT_SYMBOL(fw_iso_context_flush_completions); diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 96ae366889e0..0ae2c84ecafe 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -159,6 +159,11 @@ int fw_iso_buffer_alloc(struct fw_iso_buffer *buffer, int page_count); int fw_iso_buffer_map_dma(struct fw_iso_buffer *buffer, struct fw_card *card, enum dma_data_direction direction); +static inline void fw_iso_context_init_work(struct fw_iso_context *ctx, work_func_t func) +{ + INIT_WORK(&ctx->work, func); +} + /* -topology */ diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 02ff0363d3ad..3a911cfb5ff3 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1182,6 +1182,47 @@ static void context_tasklet(unsigned long data) } } +static void ohci_isoc_context_work(struct work_struct *work) +{ + struct fw_iso_context *base = container_of(work, struct fw_iso_context, work); + struct iso_context *isoc_ctx = container_of(base, struct iso_context, base); + struct context *ctx = &isoc_ctx->context; + struct descriptor *d, *last; + u32 address; + int z; + struct descriptor_buffer *desc; + + desc = list_entry(ctx->buffer_list.next, struct descriptor_buffer, list); + last = ctx->last; + while (last->branch_address != 0) { + struct descriptor_buffer *old_desc = desc; + + address = le32_to_cpu(last->branch_address); + z = address & 0xf; + address &= ~0xf; + ctx->current_bus = address; + + // If the branch address points to a buffer outside of the current buffer, advance + // to the next buffer. + if (address < desc->buffer_bus || address >= desc->buffer_bus + desc->used) + desc = list_entry(desc->list.next, struct descriptor_buffer, list); + d = desc->buffer + (address - desc->buffer_bus) / sizeof(*d); + last = find_branch_descriptor(d, z); + + if (!ctx->callback(ctx, d, last)) + break; + + if (old_desc != desc) { + // If we've advanced to the next buffer, move the previous buffer to the + // free list. + old_desc->used = 0; + guard(spinlock_irqsave)(&ctx->ohci->lock); + list_move_tail(&old_desc->list, &ctx->buffer_list); + } + ctx->last = last; + } +} + /* * Allocate a new buffer and add it to the list of free buffers for this * context. Must be called with ohci->lock held. @@ -3128,6 +3169,7 @@ static struct fw_iso_context *ohci_allocate_iso_context(struct fw_card *card, ret = context_init(&ctx->context, ohci, regs, callback); if (ret < 0) goto out_with_header; + fw_iso_context_init_work(&ctx->base, ohci_isoc_context_work); if (type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) { set_multichannel_mask(ohci, 0); @@ -3582,8 +3624,7 @@ static int ohci_flush_iso_completions(struct fw_iso_context *base) int ret = 0; if (!test_and_set_bit_lock(0, &ctx->flushing_completions)) { - // Note that tasklet softIRQ is not used to process isochronous context anymore. - context_tasklet((unsigned long)&ctx->context); + ohci_isoc_context_work(&base->work); switch (base->type) { case FW_ISO_CONTEXT_TRANSMIT: