* [PATCH 1/4] firewire: cdev: remove unnecessary client locking for fw_device members
2026-09-27 3:43 [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
@ 2026-09-27 3:43 ` Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 2/4] firewire: cdev: wake up client from queue_event() only when any event is available Takashi Sakamoto
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2026-09-27 3:43 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Some accesses to fw_device members unnecessarily use the client-level
lock. Remove the locking from these accesses.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-cdev.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index a626468b4b0f..5020895c42a0 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1350,17 +1350,17 @@ 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 current_generation, resource_generation, channel, bandwidth, todo;
- u64 reset_jiffies;
+ int channel, bandwidth, todo;
bool free;
- scoped_guard(spinlock_irq, &client->lock) {
- reset_jiffies = client->device->card->reset_jiffies;
- current_generation = client->device->generation;
- resource_generation = r->generation;
- r->generation = current_generation;
+ u64 reset_jiffies = client->device->card->reset_jiffies;
+ int current_generation = client->device->generation;
+
+ int resource_generation = r->generation;
+ r->generation = current_generation;
+
+ scoped_guard(spinlock_irq, &client->lock)
todo = r->todo;
- }
switch (todo) {
case ISO_RES_AUTO_ALLOC:
@@ -1514,12 +1514,10 @@ static void iso_resource_once_work(struct work_struct *work)
struct iso_resource_once *r = from_work(r, work, work);
struct client *client = r->client;
struct iso_resource_event *e = r->event;
- int generation, channel, bandwidth;
-
- scoped_guard(spinlock_irq, &client->lock)
- generation = client->device->generation;
+ int channel;
- bandwidth = r->params.bandwidth;
+ int generation = client->device->generation;
+ int bandwidth = r->params.bandwidth;
fw_iso_resource_manage(client->device->card, generation, r->params.channels_mask, &channel,
&bandwidth, r->todo == ISO_RES_ONCE_ALLOC);
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] firewire: cdev: wake up client from queue_event() only when any event is available
2026-09-27 3:43 [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 1/4] firewire: cdev: remove unnecessary client locking for fw_device members Takashi Sakamoto
@ 2026-09-27 3:43 ` Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 3/4] firewire: cdev: use xchg() to exchange pointer value Takashi Sakamoto
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2026-09-27 3:43 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
The in_shutdown member of client structure is set only when releasing
file descriptor, thus no need to wake up the client itself.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-cdev.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 5020895c42a0..9a539de9b0a1 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -338,13 +338,14 @@ static void queue_event(struct client *client, struct event *event,
event->v[1].size = size1;
scoped_guard(spinlock_irqsave, &client->lock) {
- if (client->in_shutdown)
+ if (client->in_shutdown) {
kfree(event);
- else
+ } else {
list_add_tail(&event->link, &client->event_list);
- }
- wake_up_interruptible(&client->wait);
+ wake_up_interruptible(&client->wait);
+ }
+ }
}
static int dequeue_event(struct client *client,
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] firewire: cdev: use xchg() to exchange pointer value
2026-09-27 3:43 [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 1/4] firewire: cdev: remove unnecessary client locking for fw_device members Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 2/4] firewire: cdev: wake up client from queue_event() only when any event is available Takashi Sakamoto
@ 2026-09-27 3:43 ` Takashi Sakamoto
2026-09-27 3:43 ` [PATCH 4/4] firewire: cdev: refactor event copying to remove goto statement Takashi Sakamoto
2026-09-27 22:34 ` [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2026-09-27 3:43 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
xchg() is useful for exchanging a pointer and returning the old value,
even when atomicity is not required.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-cdev.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 9a539de9b0a1..73a940d40b39 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1357,8 +1357,7 @@ static void iso_resource_auto_work(struct work_struct *work)
u64 reset_jiffies = client->device->card->reset_jiffies;
int current_generation = client->device->generation;
- int resource_generation = r->generation;
- r->generation = current_generation;
+ int resource_generation = xchg(&r->generation, current_generation); // But no need to be atomic.
scoped_guard(spinlock_irq, &client->lock)
todo = r->todo;
@@ -1388,8 +1387,7 @@ static void iso_resource_auto_work(struct work_struct *work)
if (todo == ISO_RES_AUTO_DEALLOC) {
free = true;
- e = r->e_dealloc;
- r->e_dealloc = NULL;
+ e = xchg(&r->e_dealloc, NULL); // But no need to be atomic.
} else {
free = false;
@@ -1417,8 +1415,7 @@ static void iso_resource_auto_work(struct work_struct *work)
return;
// Notify the userspace client of the failure through a deallocation event.
- e = r->e_dealloc;
- r->e_dealloc = NULL;
+ 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.
@@ -1428,8 +1425,7 @@ static void iso_resource_auto_work(struct work_struct *work)
if (channel >= 0)
r->params.channels_mask = BIT_ULL(channel);
- e = r->e_alloc;
- r->e_alloc = NULL;
+ e = xchg(&r->e_alloc, NULL); // But no need to be atomic.
}
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] firewire: cdev: refactor event copying to remove goto statement
2026-09-27 3:43 [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
` (2 preceding siblings ...)
2026-09-27 3:43 ` [PATCH 3/4] firewire: cdev: use xchg() to exchange pointer value Takashi Sakamoto
@ 2026-09-27 3:43 ` Takashi Sakamoto
2026-09-27 22:34 ` [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2026-09-27 3:43 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Arrange the local variables to avoid a goto statement when copying events
to the userspace buffer.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-cdev.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 73a940d40b39..e017086ff641 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -348,12 +348,9 @@ static void queue_event(struct client *client, struct event *event,
}
}
-static int dequeue_event(struct client *client,
- char __user *buffer, size_t count)
+static ssize_t dequeue_event(struct client *client, char __user *buffer, size_t count)
{
struct event *event;
- size_t size, total;
- int i, ret;
// After the following block, the event pointer above is guaranteed to have a correct value.
{
@@ -378,18 +375,17 @@ static int dequeue_event(struct client *client,
spin_unlock_irq(&client->lock);
}
- total = 0;
- for (i = 0; i < ARRAY_SIZE(event->v) && total < count; i++) {
- size = min(event->v[i].size, count - total);
- if (copy_to_user(buffer + total, event->v[i].data, size)) {
+ ssize_t ret = 0;
+
+ for (int i = 0; i < ARRAY_SIZE(event->v) && ret < count; i++) {
+ size_t size = min(event->v[i].size, count - ret);
+ if (copy_to_user(buffer + ret, event->v[i].data, size)) {
ret = -EFAULT;
- goto out;
+ break;
}
- total += size;
+ ret += size;
}
- ret = total;
- out:
kfree(event);
return ret;
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] firewire: cdev: code refactoring
2026-09-27 3:43 [PATCH 0/4] firewire: cdev: code refactoring Takashi Sakamoto
` (3 preceding siblings ...)
2026-09-27 3:43 ` [PATCH 4/4] firewire: cdev: refactor event copying to remove goto statement Takashi Sakamoto
@ 2026-09-27 22:34 ` Takashi Sakamoto
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2026-09-27 22:34 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
On Sun, Sep 27, 2026 at 12:43:39PM +0900, Takashi Sakamoto wrote:
> Hi,
>
> The cdev implementation includes some useless locking, goto, and waking
> up. This series arranges the implementation to remove them.
>
> Takashi Sakamoto (4):
> firewire: cdev: remove unnecessary client locking for fw_device
> members
> firewire: cdev: wake up client from queue_event() only when any event
> is available
> firewire: cdev: use xchg() to exchange pointer value
> firewire: cdev: refactor event copying to remove goto statement
>
> drivers/firewire/core-cdev.c | 61 +++++++++++++++---------------------
> 1 file changed, 26 insertions(+), 35 deletions(-)
Applied to for-next branch.
Regards
Takashi Sakamoto
^ permalink raw reply [flat|nested] 6+ messages in thread