[PATCH 0/4] firewire: core: code refactoring for isoc resource management work in cdev layer

[PATCH 0/4] firewire: core: code refactoring for isoc resource management work in cdev layer

[Takashi Sakamoto]

Hi,

In the cdev layer of this subsystem, there are two ways to manage
isochronous resources. My previous work separated the logic for these
approaches[1]. However, there is still room to improve the current
implementation, particularly in the code path that maintains
isochronous resources managed by the kernel, where the current code can
be simplified.

This patchset refactors the relevant code accordingly.


[1] https://lore.kernel.org/lkml/20260429093449.160545-1-o-takashi@sakamocchi.jp/

Takashi Sakamoto (4):
  firewire: core: reduce critical section duration in pre-processing of
    isoc resource management in cdev
  firewire: core: use switch statement for post-processing of isoc
    resource management in cdev
  firewire: core: refactor notification type determination after isoc
    resource management in cdev
  firewire: core: move allocation/reallocation paths into specific branch
    after isoc resource management in cdev

 drivers/firewire/core-cdev.c | 115 +++++++++++++++++++----------------
 1 file changed, 64 insertions(+), 51 deletions(-)


base-commit: 6dbe7653fa01edeefc77b4d7c063562eb3debd48
-- 
2.53.0

[PATCH 1/4] firewire: core: reduce critical section duration in pre-processing of isoc resource management in cdev

[Takashi Sakamoto]

It is preferable for the critical section to be as small as possible.
Current implementation of iso_resource_auto_work() function uses a
spinlock to control concurrent access to members of fw_card, fw_device,
iso_resource_auto structures, however the locking duration could be
reduced.

This commit refactors to shorten that duration.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index bcfb20b770df..887783e4bd52 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1329,32 +1329,36 @@ 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 generation, channel, bandwidth, todo;
+	int current_generation, resource_generation, channel, bandwidth, todo;
+	u64 reset_jiffies;
 	bool skip, free, success;
 
 	scoped_guard(spinlock_irq, &client->lock) {
-		generation = client->device->generation;
+		reset_jiffies = client->device->card->reset_jiffies;
+		current_generation = client->device->generation;
+		resource_generation = r->params.generation;
+		r->params.generation = current_generation;
 		todo = r->todo;
-		// Allow 1000ms grace period for other reallocations.
-		if (todo == ISO_RES_AUTO_ALLOC &&
-		    time_is_after_jiffies64(client->device->card->reset_jiffies + secs_to_jiffies(1))) {
-			schedule_iso_resource_auto(r, msecs_to_jiffies(333));
-			skip = true;
-		} else {
-			// We could be called twice within the same generation.
-			skip = todo == ISO_RES_AUTO_REALLOC &&
-			       r->params.generation == generation;
-		}
-		free = todo == ISO_RES_AUTO_DEALLOC;
-		r->params.generation = generation;
 	}
 
+	// Allow 1000ms grace period for other reallocations.
+	if (todo == ISO_RES_AUTO_ALLOC &&
+	    time_is_after_jiffies64(reset_jiffies + secs_to_jiffies(1))) {
+		schedule_iso_resource_auto(r, msecs_to_jiffies(333));
+		skip = true;
+	} else {
+		// We could be called twice within the same generation.
+		skip = todo == ISO_RES_AUTO_REALLOC &&
+		       resource_generation == current_generation;
+	}
+	free = todo == ISO_RES_AUTO_DEALLOC;
+
 	if (skip)
 		goto out;
 
 	bandwidth = r->params.bandwidth;
 
-	fw_iso_resource_manage(client->device->card, generation,
+	fw_iso_resource_manage(client->device->card, current_generation,
 			r->params.channels, &channel, &bandwidth,
 			todo == ISO_RES_AUTO_ALLOC ||
 			todo == ISO_RES_AUTO_REALLOC);
-- 
2.53.0

[PATCH 2/4] firewire: core: use switch statement for post-processing of isoc resource management in cdev

[Takashi Sakamoto]

The iso_resource_auto structure object has three states. The current
implementation of state evaluation before managing the actual isochronous
resources can be improved.

This commit refactors the evaluation logic using a switch statement.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 887783e4bd52..0d57b61ade12 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1331,7 +1331,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 	unsigned long index = r->resource.handle;
 	int current_generation, resource_generation, channel, bandwidth, todo;
 	u64 reset_jiffies;
-	bool skip, free, success;
+	bool free = false, success;
 
 	scoped_guard(spinlock_irq, &client->lock) {
 		reset_jiffies = client->device->card->reset_jiffies;
@@ -1341,27 +1341,29 @@ static void iso_resource_auto_work(struct work_struct *work)
 		todo = r->todo;
 	}
 
-	// Allow 1000ms grace period for other reallocations.
-	if (todo == ISO_RES_AUTO_ALLOC &&
-	    time_is_after_jiffies64(reset_jiffies + secs_to_jiffies(1))) {
-		schedule_iso_resource_auto(r, msecs_to_jiffies(333));
-		skip = true;
-	} else {
+	switch (todo) {
+	case ISO_RES_AUTO_ALLOC:
+		// Allow 1000ms grace period for other reallocations.
+		if (time_is_after_jiffies64(reset_jiffies + secs_to_jiffies(1))) {
+			schedule_iso_resource_auto(r, msecs_to_jiffies(333));
+			goto out;
+		}
+		break;
+	case ISO_RES_AUTO_REALLOC:
 		// We could be called twice within the same generation.
-		skip = todo == ISO_RES_AUTO_REALLOC &&
-		       resource_generation == current_generation;
+		if (resource_generation == current_generation)
+			goto out;
+		break;
+	case ISO_RES_AUTO_DEALLOC:
+	default:
+		break;
 	}
-	free = todo == ISO_RES_AUTO_DEALLOC;
-
-	if (skip)
-		goto out;
 
 	bandwidth = r->params.bandwidth;
 
-	fw_iso_resource_manage(client->device->card, current_generation,
-			r->params.channels, &channel, &bandwidth,
-			todo == ISO_RES_AUTO_ALLOC ||
-			todo == ISO_RES_AUTO_REALLOC);
+	fw_iso_resource_manage(client->device->card, current_generation, r->params.channels,
+			       &channel, &bandwidth, todo != ISO_RES_AUTO_DEALLOC);
+
 	/*
 	 * Is this generation outdated already?  As long as this resource sticks
 	 * in the xarray, it will be scheduled again for a newer generation or at
@@ -1398,6 +1400,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 		e = r->e_alloc;
 		r->e_alloc = NULL;
 	} else {
+		free = true;
 		e = r->e_dealloc;
 		r->e_dealloc = NULL;
 	}
-- 
2.53.0

[PATCH 3/4] firewire: core: refactor notification type determination after isoc resource management in cdev

[Takashi Sakamoto]

After managing the actual isochronous resources, there is
post-processing logic to determine what type of event should be
notified. However, there is room for improvement.

This commit refactors the logic.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 0d57b61ade12..4ce8754da93f 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1390,20 +1390,27 @@ static void iso_resource_auto_work(struct work_struct *work)
 		}
 	}
 
-	if (todo == ISO_RES_AUTO_ALLOC && channel >= 0)
-		r->params.channels = 1ULL << channel;
-
-	if (todo == ISO_RES_AUTO_REALLOC && success)
-		goto out;
-
-	if (todo == ISO_RES_AUTO_ALLOC) {
-		e = r->e_alloc;
-		r->e_alloc = NULL;
-	} else {
+	if (todo == ISO_RES_AUTO_DEALLOC) {
 		free = true;
 		e = r->e_dealloc;
 		r->e_dealloc = NULL;
+	} else {
+		if (todo == ISO_RES_AUTO_REALLOC) {
+			if (success)
+				goto out;
+
+			// Notify the userspace client of the failure through a deallocation event.
+			e = r->e_dealloc;
+			r->e_dealloc = NULL;
+		} else {
+			if (channel >= 0)
+				r->params.channels = 1ULL << channel;
+
+			e = r->e_alloc;
+			r->e_alloc = NULL;
+		}
 	}
+
 	e->iso_resource.handle    = r->resource.handle;
 	e->iso_resource.channel   = channel;
 	e->iso_resource.bandwidth = bandwidth;
-- 
2.53.0

[PATCH 4/4] firewire: core: move allocation/reallocation paths into specific branch after isoc resource management in cdev

[Takashi Sakamoto]

After managing the actual isochronous resources, there is
post-processing logic to determine what type of event should be
notified. However, there is room for improvement.

This commit refactors the logic.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 4ce8754da93f..c166e7617d2a 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1331,7 +1331,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 	unsigned long index = r->resource.handle;
 	int current_generation, resource_generation, channel, bandwidth, todo;
 	u64 reset_jiffies;
-	bool free = false, success;
+	bool free;
 
 	scoped_guard(spinlock_irq, &client->lock) {
 		reset_jiffies = client->device->card->reset_jiffies;
@@ -1364,37 +1364,31 @@ static void iso_resource_auto_work(struct work_struct *work)
 	fw_iso_resource_manage(client->device->card, current_generation, r->params.channels,
 			       &channel, &bandwidth, todo != ISO_RES_AUTO_DEALLOC);
 
-	/*
-	 * Is this generation outdated already?  As long as this resource sticks
-	 * in the xarray, it will be scheduled again for a newer generation or at
-	 * shutdown.
-	 */
-	if (channel == -EAGAIN &&
-	    (todo == ISO_RES_AUTO_ALLOC || todo == ISO_RES_AUTO_REALLOC))
-		goto out;
-
-	success = channel >= 0 || bandwidth > 0;
-
-	scoped_guard(spinlock_irq, &client->lock) {
-		// Transit from allocation to reallocation, except if the client
-		// requested deallocation in the meantime.
-		if (r->todo == ISO_RES_AUTO_ALLOC)
-			r->todo = ISO_RES_AUTO_REALLOC;
-		// Allocation or reallocation failure?  Pull this resource out of the
-		// xarray and prepare for deletion, unless the client is shutting down.
-		if (r->todo == ISO_RES_AUTO_REALLOC && !success &&
-		    !client->in_shutdown &&
-		    xa_erase(&client->resource_xa, index)) {
-			client_put(client);
-			free = true;
-		}
-	}
-
 	if (todo == ISO_RES_AUTO_DEALLOC) {
 		free = true;
 		e = r->e_dealloc;
 		r->e_dealloc = NULL;
 	} else {
+		free = false;
+
+		// Is this generation outdated already?  As long as this resource sticks in the
+		// xarray, it will be scheduled again for a newer generation or at shutdown.
+		if (channel == -EAGAIN)
+			goto out;
+
+		bool success = channel >= 0 || bandwidth > 0;
+
+		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) {
+				if (!client->in_shutdown && xa_erase(&client->resource_xa, index)) {
+					client_put(client);
+					free = true;
+				}
+			}
+		}
+
 		if (todo == ISO_RES_AUTO_REALLOC) {
 			if (success)
 				goto out;
@@ -1403,6 +1397,11 @@ static void iso_resource_auto_work(struct work_struct *work)
 			e = r->e_dealloc;
 			r->e_dealloc = NULL;
 		} else {
+			// Transit from allocation to reallocation, except if the client requested
+			// deallocation in the meantime.
+			scoped_guard(spinlock_irq,  &client->lock)
+				r->todo = ISO_RES_AUTO_REALLOC;
+
 			if (channel >= 0)
 				r->params.channels = 1ULL << channel;
 
-- 
2.53.0

Re: [PATCH 0/4] firewire: core: code refactoring for isoc resource management work in cdev layer

[Takashi Sakamoto]

On Fri, May 01, 2026 at 10:58:19PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> In the cdev layer of this subsystem, there are two ways to manage
> isochronous resources. My previous work separated the logic for these
> approaches[1]. However, there is still room to improve the current
> implementation, particularly in the code path that maintains
> isochronous resources managed by the kernel, where the current code can
> be simplified.
> 
> This patchset refactors the relevant code accordingly.
> 
> 
> [1] https://lore.kernel.org/lkml/20260429093449.160545-1-o-takashi@sakamocchi.jp/
> 
> Takashi Sakamoto (4):
>   firewire: core: reduce critical section duration in pre-processing of
>     isoc resource management in cdev
>   firewire: core: use switch statement for post-processing of isoc
>     resource management in cdev
>   firewire: core: refactor notification type determination after isoc
>     resource management in cdev
>   firewire: core: move allocation/reallocation paths into specific branch
>     after isoc resource management in cdev
> 
>  drivers/firewire/core-cdev.c | 115 +++++++++++++++++++----------------
>  1 file changed, 64 insertions(+), 51 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto