[PATCH 2/2] firewire: core: use mutex to coordinate concurrent calls to flush completions

From: Takashi Sakamoto
Date: Mon Sep 09 2024 - 10:02:31 EST


In current implementation, test_and_set_bit_lock() is used to mediate
concurrent calls of ohci_flush_iso_completions(). However, the ad-hoc
usage of atomic operations is not preferable.

This commit uses mutex_trylock() as the similar operations. The core
function is responsible for the mediation, instead of 1394 OHCI driver.

Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
---
drivers/firewire/core-iso.c | 11 +++++++++--
drivers/firewire/ohci.c | 37 ++++++++++++++-----------------------
include/linux/firewire.h | 1 +
3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 9f41c78878ad..a5c5ef3c725d 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -157,6 +157,7 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
ctx->callback.sc = callback;
ctx->callback_data = callback_data;
INIT_WORK(&ctx->work, flush_completions_work);
+ mutex_init(&ctx->flushing_completions_mutex);

trace_isoc_outbound_allocate(ctx, channel, speed);
trace_isoc_inbound_single_allocate(ctx, channel, header_size);
@@ -173,6 +174,8 @@ void fw_iso_context_destroy(struct fw_iso_context *ctx)
trace_isoc_inbound_multiple_destroy(ctx);

ctx->card->driver->free_iso_context(ctx);
+
+ mutex_destroy(&ctx->flushing_completions_mutex);
}
EXPORT_SYMBOL(fw_iso_context_destroy);

@@ -226,7 +229,7 @@ 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 due to mutex_trylock().
*/
int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
{
@@ -234,7 +237,11 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
trace_isoc_inbound_single_flush_completions(ctx);
trace_isoc_inbound_multiple_flush_completions(ctx);

- return ctx->card->driver->flush_iso_completions(ctx);
+ scoped_cond_guard(mutex_try, /* nothing to do */, &ctx->flushing_completions_mutex) {
+ return ctx->card->driver->flush_iso_completions(ctx);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(fw_iso_context_flush_completions);

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 02ff0363d3ad..b182998a77f4 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -166,7 +166,6 @@ struct iso_context {
struct context context;
void *header;
size_t header_length;
- unsigned long flushing_completions;
u32 mc_buffer_bus;
u16 mc_completed;
u16 last_timestamp;
@@ -3579,31 +3578,23 @@ static void ohci_flush_queue_iso(struct fw_iso_context *base)
static int ohci_flush_iso_completions(struct fw_iso_context *base)
{
struct iso_context *ctx = container_of(base, struct 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);
+ // Note that tasklet softIRQ is not used to process isochronous context anymore.
+ context_tasklet((unsigned long)&ctx->context);

- switch (base->type) {
- case FW_ISO_CONTEXT_TRANSMIT:
- case FW_ISO_CONTEXT_RECEIVE:
- if (ctx->header_length != 0)
- flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
- break;
- case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
- if (ctx->mc_completed != 0)
- flush_ir_buffer_fill(ctx);
- break;
- default:
- ret = -ENOSYS;
- }
-
- clear_bit_unlock(0, &ctx->flushing_completions);
- smp_mb__after_atomic();
+ switch (base->type) {
+ case FW_ISO_CONTEXT_TRANSMIT:
+ case FW_ISO_CONTEXT_RECEIVE:
+ if (ctx->header_length != 0)
+ flush_iso_completions(ctx, FW_ISO_CONTEXT_COMPLETIONS_CAUSE_FLUSH);
+ return 0;
+ case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
+ if (ctx->mc_completed != 0)
+ flush_ir_buffer_fill(ctx);
+ return 0;
+ default:
+ return -ENOSYS;
}
-
- return ret;
}

static const struct fw_card_driver ohci_driver = {
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index f815d12deda0..19e8c5f9537c 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -512,6 +512,7 @@ union fw_iso_callback {
struct fw_iso_context {
struct fw_card *card;
struct work_struct work;
+ struct mutex flushing_completions_mutex;
int type;
int channel;
int speed;
--
2.43.0