[PATCH 0/2] firewire: cdev: replace spin lock with mutex

[PATCH 0/2] firewire: cdev: replace spin lock with mutex

[Takashi Sakamoto]

Hi,

All execution paths in the cdev layer now run in process context.

This series replaces the spin lock with mutex to avoid CPU consumption
while waiting for the lock.


Takashi Sakamoto (2):
  firewire: cdev: use atomic_t for iso_resource_auto todo member
  firewire: cdev: use mutex for client locking

 drivers/firewire/core-cdev.c | 89 ++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 44 deletions(-)


base-commit: a781263c292e7bda50812ba2347459e8ce9d778b
-- 
2.53.0

[PATCH 1/2] firewire: cdev: use atomic_t for iso_resource_auto todo member

[Takashi Sakamoto]

The transition state of an iso_resource_auto client resource has no
effect on the client, so the client-level lock is not suitable for
protecting it.

At present, this state is the only member that requires mutual
exclusion. Adding a separate lock for it would be excessive.

Change the type of the transition state to atomic_t and use an atomic
compare-and-swap operation to preserve consistency.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-cdev.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index a62df9034f4f..d4c72cd20069 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -133,16 +133,17 @@ struct iso_resource_params {
 	s32 bandwidth;
 };
 
+enum {
+	ISO_RES_AUTO_ALLOC,
+	ISO_RES_AUTO_REALLOC,
+	ISO_RES_AUTO_DEALLOC,
+};
+
 struct iso_resource_auto {
 	struct client_resource resource;
 	struct client *client;
-	/* Schedule work and access todo only with client->lock held. */
 	struct delayed_work work;
-	enum {
-		ISO_RES_AUTO_ALLOC,
-		ISO_RES_AUTO_REALLOC,
-		ISO_RES_AUTO_DEALLOC,
-	} todo;
+	atomic_t todo; // one of ISO_RES_AUTO_XXX.
 	int generation;
 	struct iso_resource_params params;
 	struct iso_resource_event *e_alloc, *e_dealloc;
@@ -1338,16 +1339,14 @@ static void iso_resource_auto_work(struct work_struct *work)
 	struct iso_resource_auto *r = from_work(r, work, work.work);
 	struct client *client = r->client;
 	unsigned long index = r->resource.handle;
-	int channel, bandwidth, todo;
+	int channel, bandwidth;
 	bool free;
 
 	u64 reset_jiffies = client->device->card->reset_jiffies;
 	int current_generation = client->device->generation;
 
 	int resource_generation = xchg(&r->generation, current_generation); // But no need to be atomic.
-
-	scoped_guard(spinlock_irq, &client->lock)
-		todo = r->todo;
+	int todo = atomic_read(&r->todo);
 
 	switch (todo) {
 	case ISO_RES_AUTO_ALLOC:
@@ -1404,12 +1403,10 @@ static void iso_resource_auto_work(struct work_struct *work)
 			// Notify the userspace client of the failure through a deallocation event.
 			e = xchg(&r->e_dealloc, NULL); // But no need to be atomic.
 		} else {
-			// Transit from allocation to reallocation, except if the client requested
-			// deallocation in the meantime.
-			scoped_guard(spinlock_irq,  &client->lock) {
-				if (r->todo == ISO_RES_AUTO_ALLOC)
-					r->todo = ISO_RES_AUTO_REALLOC;
-			}
+			// Transit from allocation to reallocation. Use compare-and-swap atomic
+			// operation because the todo member can be set with ISO_RES_AUTO_DEALLOC
+			// by release_iso_resource_auto() in parallel.
+			atomic_cmpxchg_relaxed(&r->todo, ISO_RES_AUTO_ALLOC, ISO_RES_AUTO_REALLOC);
 
 			if (channel >= 0)
 				r->params.channels_mask = BIT_ULL(channel);
@@ -1440,9 +1437,7 @@ static void release_iso_resource_auto(struct client *client, struct client_resou
 {
 	struct iso_resource_auto *r = to_iso_resource_auto(resource);
 
-	guard(spinlock_irq)(&client->lock);
-
-	r->todo = ISO_RES_AUTO_DEALLOC;
+	atomic_set(&r->todo, ISO_RES_AUTO_DEALLOC);
 	schedule_iso_resource_auto(r, 0);
 }
 
@@ -1463,7 +1458,7 @@ static int ioctl_allocate_iso_resource(struct client *client, union ioctl_arg *a
 
 	INIT_DELAYED_WORK(&r->work, iso_resource_auto_work);
 	r->client	= client;
-	r->todo		= ISO_RES_AUTO_ALLOC;
+	atomic_set(&r->todo, ISO_RES_AUTO_ALLOC);
 	r->e_alloc	= e1;
 	r->e_dealloc	= e2;
 
-- 
2.53.0

[PATCH 2/2] firewire: cdev: use mutex for client locking

[Takashi Sakamoto]

All execution paths in the cdev layer now run in process context.

Use a mutex instead of a spinlock for client-level locking.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-cdev.c | 54 ++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index d4c72cd20069..9c204bd7af74 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -54,7 +54,7 @@ struct client {
 	u32 version;
 	struct fw_device *device;
 
-	spinlock_t lock;
+	struct mutex mutex;
 	bool in_shutdown;
 	struct xarray resource_xa;
 	struct list_head event_list;
@@ -315,7 +315,7 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
 	}
 
 	client->device = device;
-	spin_lock_init(&client->lock);
+	mutex_init(&client->mutex);
 	xa_init_flags(&client->resource_xa, XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH);
 	INIT_LIST_HEAD(&client->event_list);
 	init_waitqueue_head(&client->wait);
@@ -338,7 +338,7 @@ static void queue_event(struct client *client, struct event *event,
 	event->v[1].data = data1;
 	event->v[1].size = size1;
 
-	scoped_guard(spinlock_irqsave, &client->lock) {
+	scoped_guard(mutex, &client->mutex) {
 		if (client->in_shutdown) {
 			kfree(event);
 		} else {
@@ -355,25 +355,31 @@ static ssize_t dequeue_event(struct client *client, char __user *buffer, size_t
 
 	// After the following block, the event pointer above is guaranteed to have a correct value.
 	{
-		spin_lock_irq(&client->lock);
+		mutex_lock(&client->mutex);
 
-		int ret = wait_event_interruptible_lock_irq(client->wait,
+		// This could be replaced with wait_var_event_any_lock() if poll_wait() alternative
+		// would be introduced.
+		int ret = ___wait_event(client->wait,
 			!list_empty(&client->event_list) || fw_device_is_shutdown(client->device),
-			client->lock);
+			TASK_INTERRUPTIBLE, 0, 0,
+			mutex_unlock(&client->mutex);
+			schedule();
+			mutex_lock(&client->mutex)
+		);
 		if (ret < 0) {
-			spin_unlock_irq(&client->lock);
+			mutex_unlock(&client->mutex);
 			return ret;
 		}
 
 		if (fw_device_is_shutdown(client->device)) {
-			spin_unlock_irq(&client->lock);
+			mutex_unlock(&client->mutex);
 			return -ENODEV;
 		}
 
 		event = list_first_entry(&client->event_list, struct event, link);
 		list_del(&event->link);
 
-		spin_unlock_irq(&client->lock);
+		mutex_unlock(&client->mutex);
 	}
 
 	ssize_t ret = 0;
@@ -451,11 +457,11 @@ static void queue_bus_reset_event(struct client *client)
 	queue_event(client, &e->event,
 		    &e->reset, sizeof(e->reset), NULL, 0);
 
-	guard(spinlock_irq)(&client->lock);
-
-	xa_for_each(&client->resource_xa, index, resource) {
-		if (is_iso_resource_auto(resource))
-			schedule_iso_resource_auto(to_iso_resource_auto(resource), 0);
+	scoped_guard(mutex, &client->mutex) {
+		xa_for_each(&client->resource_xa, index, resource) {
+			if (is_iso_resource_auto(resource))
+				schedule_iso_resource_auto(to_iso_resource_auto(resource), 0);
+		}
 	}
 }
 
@@ -546,7 +552,7 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
 static int add_client_resource(struct client *client, struct client_resource *resource,
 			       client_resource_release_fn_t release)
 {
-	scoped_guard(spinlock_irqsave, &client->lock) {
+	scoped_guard(mutex, &client->mutex) {
 		u32 index;
 		int ret;
 
@@ -572,7 +578,7 @@ static int release_client_resource(struct client *client, u32 handle,
 	unsigned long index = handle;
 	struct client_resource *resource;
 
-	scoped_guard(spinlock_irq, &client->lock) {
+	scoped_guard(mutex, &client->mutex) {
 		if (client->in_shutdown)
 			return -EINVAL;
 
@@ -605,7 +611,7 @@ static void complete_transaction(struct fw_card *card, int rcode, u32 request_ts
 	struct client *client = e->client;
 	unsigned long index = e->r.resource.handle;
 
-	scoped_guard(spinlock_irqsave, &client->lock) {
+	scoped_guard(mutex, &client->mutex) {
 		xa_erase(&client->resource_xa, index);
 		if (client->in_shutdown)
 			wake_up(&client->tx_flush_wait);
@@ -1387,7 +1393,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 		if (!success) {
 			// Allocation or reallocation failure?  Pull this resource out of the
 			// xarray and prepare for deletion, unless the client is shutting down.
-			scoped_guard(spinlock_irq,  &client->lock) {
+			scoped_guard(mutex,  &client->mutex) {
 				if (!client->in_shutdown && xa_erase(&client->resource_xa, index)) {
 					// For the incrementation by add_client_resource().
 					client_put(client);
@@ -1907,11 +1913,11 @@ static bool has_outbound_transactions(struct client *client)
 	struct client_resource *resource;
 	unsigned long index;
 
-	guard(spinlock_irq)(&client->lock);
-
-	xa_for_each(&client->resource_xa, index, resource) {
-		if (is_outbound_transaction_resource(resource))
-			return true;
+	scoped_guard(mutex, &client->mutex) {
+		xa_for_each(&client->resource_xa, index, resource) {
+			if (is_outbound_transaction_resource(resource))
+				return true;
+		}
 	}
 
 	return false;
@@ -1938,7 +1944,7 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
 		fw_iso_buffer_destroy(&client->buffer, client->device->card);
 
 	// Freeze client->resource_xa and client->event_list.
-	scoped_guard(spinlock_irq, &client->lock)
+	scoped_guard(mutex, &client->mutex)
 		client->in_shutdown = true;
 
 	wait_event(client->tx_flush_wait, !has_outbound_transactions(client));
-- 
2.53.0

Re: [PATCH 0/2] firewire: cdev: replace spin lock with mutex

[Takashi Sakamoto]

On Mon, Sep 28, 2026 at 08:07:22AM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> All execution paths in the cdev layer now run in process context.
> 
> This series replaces the spin lock with mutex to avoid CPU consumption
> while waiting for the lock.
> 
> 
> Takashi Sakamoto (2):
>   firewire: cdev: use atomic_t for iso_resource_auto todo member
>   firewire: cdev: use mutex for client locking
> 
>  drivers/firewire/core-cdev.c | 89 ++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 44 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto