[PATCH 0/5] firewire: cdev: refactor packet callbacks in process context

[PATCH 0/5] firewire: cdev: refactor packet callbacks in process context

[Takashi Sakamoto]

Hi,

The address handlers in the cdev layer are now invoked in process context,
following the change to use a workqueue for all packet callbacks in the
1394 OHCI driver[1]. This allows the cdev layer to use GFP_KERNEL for
memory allocation and replace a spinlock with a mutex where appropriate.

This series first updates the kerneldoc for the address handler, and then
refactors the cdev layer accordingly.

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


Takashi Sakamoto (5):
  firewire: core: fulfill kerneldoc for address handler
  firewire: cdev: use mutex for phy receiver list
  firewire: cdev: use GFP_KERNEL in address handler
  firewire: cdev: refactor add_client_resource() to drop GFP flags
    argument
  firewire: cdev: refactor add_client_resource() to have release
    callback function

 drivers/firewire/core-cdev.c        | 50 +++++++++--------------------
 drivers/firewire/core-transaction.c |  8 ++---
 include/linux/firewire.h            | 22 +++++++++----
 3 files changed, 34 insertions(+), 46 deletions(-)


base-commit: d34653cbe03fb090325a61566348cabc2d9c83df
-- 
2.53.0

[PATCH 1/5] firewire: core: fulfill kerneldoc for address handler

[Takashi Sakamoto]

The callback for the address handler is now invoked in process context.

Update the kerneldoc for the address handler accordingly.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c |  8 +++-----
 include/linux/firewire.h            | 22 +++++++++++++++-------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index a2a8d755ad0a..bdd2437c6f7f 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -637,16 +637,14 @@ static int put_address_handler(struct fw_address_handler *handler)
  *
  * When a request is received that falls within the specified address range, the specified callback
  * is invoked.  The parameters passed to the callback give the details of the particular request.
- * The callback is invoked in the workqueue context in most cases. However, if the request is
- * initiated by the local node, the callback is invoked in the initiator's context.
- *
- * To be called in process context.
- * Return value:  0 on success, non-zero otherwise.
  *
  * The start offset of the handler's address region is determined by
  * fw_core_add_address_handler() and is returned in handler->offset.
  *
  * Address allocations are exclusive, except for the FCP registers.
+ *
+ * Context: Process context.
+ * Returns: 0 on success, non-zero otherwise.
  */
 int fw_core_add_address_handler(struct fw_address_handler *handler,
 				const struct fw_address_region *region)
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 2b065f03565d..4bca5af6793d 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -298,17 +298,25 @@ union fw_transaction_callback {
 	fw_transaction_callback_with_tstamp_t with_tstamp;
 };
 
-/*
- * This callback handles an inbound request subaction. If the request subaction is initiated from
- * the local node (e.g. by unit driver), the execution context depends on the initiator and is
- * unspecified. Otherwise, it runs in workqueue context.
+/**
+ * typedef fw_address_callback_t - Function to handle the request of the asynchronous transaction.
+ * @card: the card instance which receives the request
+ * @request: the request instance.
+ * @tcode: the transaction code
+ * @destination: the destination node ID
+ * @source: the source node ID
+ * @generation: the bus generation in which the request was sent
+ * @offset: the destination offset in source node.
+ * @data: the request content if available.
+ * @length: the length of data.
+ * @callback_data: the data registered with this function.
  *
- * The callback should not initiate outbound request subactions directly.
- * Otherwise there is a danger of recursion of inbound and outbound
- * transactions from and to the local node.
+ * This callback handles an inbound request subaction.
  *
  * The callback is responsible that fw_send_response() is called on the @request, except for FCP
  * registers for which the core takes care of that.
+ *
+ * Context: Process context.
  */
 typedef void (*fw_address_callback_t)(struct fw_card *card,
 				      struct fw_request *request,
-- 
2.53.0

[PATCH 2/5] firewire: cdev: use mutex for phy receiver list

[Takashi Sakamoto]

In the cdev layer, the list of phy packet receivers is currently
protected by a spinlock. This requires GFP_ATOMIC for memory allocation
when enumerating the list in fw_cdev_handle_phy_packet(). However, the
function is guaranteed to be invoked in process context, so GFP_KERNEL
can be used by switching from the spinlock to a mutex.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 6d91a6e7ad0d..3dd8ea1991b0 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -47,7 +47,7 @@
 #define FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW	5
 #define FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP	6
 
-static DEFINE_SPINLOCK(phy_receiver_list_lock);
+static DEFINE_MUTEX(phy_receiver_list_mutex);
 static LIST_HEAD(phy_receiver_list);
 
 struct client {
@@ -1751,9 +1751,7 @@ static int ioctl_receive_phy_packets(struct client *client, union ioctl_arg *arg
 	if (!client->device->is_local)
 		return -ENOSYS;
 
-	// NOTE: This can be without irq when we can guarantee that __fw_send_request() for local
-	// destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irq, &phy_receiver_list_lock)
+	scoped_guard(mutex, &phy_receiver_list_mutex)
 		list_move_tail(&client->phy_receiver_link, &phy_receiver_list);
 
 	client->phy_receiver_closure = a->closure;
@@ -1765,18 +1763,14 @@ void fw_cdev_handle_phy_packet(struct fw_card *card, struct fw_packet *p)
 {
 	struct client *client;
 
-	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for local
-	// destination never runs in any type of IRQ context.
-	guard(spinlock_irqsave)(&phy_receiver_list_lock);
+	guard(mutex)(&phy_receiver_list_mutex);
 
 	list_for_each_entry(client, &phy_receiver_list, phy_receiver_link) {
-		struct inbound_phy_packet_event *e;
-
 		if (client->device->card != card)
 			continue;
 
-		e = kmalloc(sizeof(*e) + 8, GFP_ATOMIC);
-		if (e == NULL)
+		struct inbound_phy_packet_event *e = kmalloc(sizeof(*e) + 8, GFP_KERNEL);
+		if (!e)
 			break;
 
 		if (client->version < FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP) {
@@ -1943,9 +1937,7 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
 	struct client_resource *resource;
 	unsigned long index;
 
-	// NOTE: This can be without irq when we can guarantee that __fw_send_request() for local
-	// destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irq, &phy_receiver_list_lock)
+	scoped_guard(mutex, &phy_receiver_list_mutex)
 		list_del(&client->phy_receiver_link);
 
 	scoped_guard(mutex, &client->device->client_list_mutex)
-- 
2.53.0

[PATCH 3/5] firewire: cdev: use GFP_KERNEL in address handler

[Takashi Sakamoto]

The callback for the address handler is now guaranteed to be invoked in
process context. In the cdev layer, GFP_ATOMIC is still used for memory
allocation in the address handler.

Use GFP_KERNEL instead.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 3dd8ea1991b0..625792b6ab71 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -777,8 +777,8 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
 	if (is_fcp)
 		fw_request_get(request);
 
-	r = kmalloc_obj(*r, GFP_ATOMIC);
-	e = kmalloc_obj(*e, GFP_ATOMIC);
+	r = kmalloc_obj(*r);
+	e = kmalloc_obj(*e);
 	if (r == NULL || e == NULL)
 		goto failed;
 
@@ -789,7 +789,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
 	r->length  = length;
 
 	r->resource.release = release_request;
-	ret = add_client_resource(handler->client, &r->resource, GFP_ATOMIC);
+	ret = add_client_resource(handler->client, &r->resource, GFP_KERNEL);
 	if (ret < 0)
 		goto failed;
 
-- 
2.53.0

[PATCH 4/5] firewire: cdev: refactor add_client_resource() to drop GFP flags argument

[Takashi Sakamoto]

The add_client_resource() function has an argument for GFP flags. The
argument was originally used to distinguish atomic calls. However, all
callers now use GFP_KERNEL.

Drop the unused argument and always allocate with GFP_KERNEL.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 625792b6ab71..96f3619237f3 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -539,8 +539,7 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
 	return 0;
 }
 
-static int add_client_resource(struct client *client, struct client_resource *resource,
-			       gfp_t gfp_mask)
+static int add_client_resource(struct client *client, struct client_resource *resource)
 {
 	scoped_guard(spinlock_irqsave, &client->lock) {
 		u32 index;
@@ -549,13 +548,7 @@ static int add_client_resource(struct client *client, struct client_resource *re
 		if (client->in_shutdown)
 			return  -ECANCELED;
 
-		if (gfpflags_allow_blocking(gfp_mask)) {
-			ret = xa_alloc(&client->resource_xa, &index, resource, xa_limit_32b,
-				       GFP_NOWAIT);
-		} else {
-			ret = xa_alloc_bh(&client->resource_xa, &index, resource,
-					  xa_limit_32b, GFP_NOWAIT);
-		}
+		ret = xa_alloc(&client->resource_xa, &index, resource, xa_limit_32b, GFP_KERNEL);
 		if (ret < 0)
 			return ret;
 
@@ -705,7 +698,7 @@ static int init_request(struct client *client,
 	}
 
 	e->r.resource.release = release_transaction;
-	ret = add_client_resource(client, &e->r.resource, GFP_KERNEL);
+	ret = add_client_resource(client, &e->r.resource);
 	if (ret < 0)
 		goto failed;
 
@@ -789,7 +782,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
 	r->length  = length;
 
 	r->resource.release = release_request;
-	ret = add_client_resource(handler->client, &r->resource, GFP_KERNEL);
+	ret = add_client_resource(handler->client, &r->resource);
 	if (ret < 0)
 		goto failed;
 
@@ -893,7 +886,7 @@ static int ioctl_allocate(struct client *client, union ioctl_arg *arg)
 	a->offset = r->handler.offset;
 
 	r->resource.release = release_address_handler;
-	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+	ret = add_client_resource(client, &r->resource);
 	if (ret < 0) {
 		release_address_handler(client, &r->resource);
 		return ret;
@@ -993,7 +986,7 @@ static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 		goto failed;
 
 	r->resource.release = release_descriptor;
-	ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+	ret = add_client_resource(client, &r->resource);
 	if (ret < 0) {
 		fw_core_remove_descriptor(&r->descriptor);
 		goto failed;
@@ -1479,7 +1472,7 @@ static int ioctl_allocate_iso_resource(struct client *client, union ioctl_arg *a
 	e2->iso_resource.type = FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
 
 	r->resource.release = release_iso_resource_auto;
-	err = add_client_resource(client, &r->resource, GFP_KERNEL);
+	err = add_client_resource(client, &r->resource);
 	if (err < 0)
 		return err;
 	request->handle = r->resource.handle;
-- 
2.53.0

[PATCH 5/5] firewire: cdev: refactor add_client_resource() to have release callback function

[Takashi Sakamoto]

The implementation of the client_resource structure now requires a release
callback.

Add an argument to add_client_resource() for the release callback.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 96f3619237f3..9a471307aa5d 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -539,7 +539,8 @@ static int ioctl_get_info(struct client *client, union ioctl_arg *arg)
 	return 0;
 }
 
-static int add_client_resource(struct client *client, struct client_resource *resource)
+static int add_client_resource(struct client *client, struct client_resource *resource,
+			       client_resource_release_fn_t release)
 {
 	scoped_guard(spinlock_irqsave, &client->lock) {
 		u32 index;
@@ -553,6 +554,7 @@ static int add_client_resource(struct client *client, struct client_resource *re
 			return ret;
 
 		resource->handle = index;
+		resource->release = release;
 		client_get(client);
 	}
 
@@ -697,8 +699,7 @@ static int init_request(struct client *client,
 		goto failed;
 	}
 
-	e->r.resource.release = release_transaction;
-	ret = add_client_resource(client, &e->r.resource);
+	ret = add_client_resource(client, &e->r.resource, release_transaction);
 	if (ret < 0)
 		goto failed;
 
@@ -781,8 +782,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
 	r->data    = payload;
 	r->length  = length;
 
-	r->resource.release = release_request;
-	ret = add_client_resource(handler->client, &r->resource);
+	ret = add_client_resource(handler->client, &r->resource, release_request);
 	if (ret < 0)
 		goto failed;
 
@@ -885,8 +885,7 @@ static int ioctl_allocate(struct client *client, union ioctl_arg *arg)
 	}
 	a->offset = r->handler.offset;
 
-	r->resource.release = release_address_handler;
-	ret = add_client_resource(client, &r->resource);
+	ret = add_client_resource(client, &r->resource, release_address_handler);
 	if (ret < 0) {
 		release_address_handler(client, &r->resource);
 		return ret;
@@ -985,8 +984,7 @@ static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 	if (ret < 0)
 		goto failed;
 
-	r->resource.release = release_descriptor;
-	ret = add_client_resource(client, &r->resource);
+	ret = add_client_resource(client, &r->resource, release_descriptor);
 	if (ret < 0) {
 		fw_core_remove_descriptor(&r->descriptor);
 		goto failed;
@@ -1471,8 +1469,7 @@ static int ioctl_allocate_iso_resource(struct client *client, union ioctl_arg *a
 	e2->iso_resource.closure = request->closure;
 	e2->iso_resource.type = FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
 
-	r->resource.release = release_iso_resource_auto;
-	err = add_client_resource(client, &r->resource);
+	err = add_client_resource(client, &r->resource, release_iso_resource_auto);
 	if (err < 0)
 		return err;
 	request->handle = r->resource.handle;
-- 
2.53.0

Re: [PATCH 0/5] firewire: cdev: refactor packet callbacks in process context

[Takashi Sakamoto]

On Sun, Sep 20, 2026 at 05:36:54PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> The address handlers in the cdev layer are now invoked in process context,
> following the change to use a workqueue for all packet callbacks in the
> 1394 OHCI driver[1]. This allows the cdev layer to use GFP_KERNEL for
> memory allocation and replace a spinlock with a mutex where appropriate.
> 
> This series first updates the kerneldoc for the address handler, and then
> refactors the cdev layer accordingly.
> 
> [1] https://lore.kernel.org/lkml/20260919115017.859169-1-o-takashi@sakamocchi.jp/
> 
> 
> Takashi Sakamoto (5):
>   firewire: core: fulfill kerneldoc for address handler
>   firewire: cdev: use mutex for phy receiver list
>   firewire: cdev: use GFP_KERNEL in address handler
>   firewire: cdev: refactor add_client_resource() to drop GFP flags
>     argument
>   firewire: cdev: refactor add_client_resource() to have release
>     callback function
> 
>  drivers/firewire/core-cdev.c        | 50 +++++++++--------------------
>  drivers/firewire/core-transaction.c |  8 ++---
>  include/linux/firewire.h            | 22 +++++++++----
>  3 files changed, 34 insertions(+), 46 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto