mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] firewire: ohci: use regular workqueue to handle 1394 OHCI AT/AR context events
@ 2025-06-14 11:34 Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts Takashi Sakamoto
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2025-06-14 11:34 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Hi,

Last year, in Linux kernel v6.12, the bottom-halves for isochronous
contexts of 1394 OHCI PCI driver were changed to use workqueue instead of
tasklet (softIRQ). I have received no reports of any issues related to the
change until today. Therefore, I believe it's time to move on to the next
step.

This patchset updates the driver to use a regular workqueue (not WQ_BH) to
handle 1394 OHCI AT/AR context events. Unlike isochronous contexts, the
asynchronous contexts are used by the implementation of the SCSI over
IEEE 1394 protocol (sbp2). The workqueue is allocated with WQ_MEM_RECLAIM
flag so that it can still participate in memory reclaim paths.

With this change, all remaining uses of tasklets in the subsystem are
completely removed.

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

Takashi Sakamoto (3):
  firewire: core: allocate workqueue for AR/AT request/response contexts
  firewire: ohci: use workqueue to handle events of AR request/response
    contexts
  firewire: ohci: use workqueue to handle events of AT request/response
    contexts

 drivers/firewire/core-card.c        | 40 ++++++++++++-----
 drivers/firewire/core-transaction.c |  7 +--
 drivers/firewire/net.c              |  4 +-
 drivers/firewire/ohci.c             | 67 +++++++++++++++--------------
 include/linux/firewire.h            | 12 +++++-
 5 files changed, 81 insertions(+), 49 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts
  2025-06-14 11:34 [PATCH 0/3] firewire: ohci: use regular workqueue to handle 1394 OHCI AT/AR context events Takashi Sakamoto
@ 2025-06-14 11:34 ` Takashi Sakamoto
  2025-06-15  4:40   ` kernel test robot
  2025-06-14 11:34 ` [PATCH 2/3] firewire: ohci: use workqueue to handle events of AR " Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 3/3] firewire: ohci: use workqueue to handle events of AT " Takashi Sakamoto
  2 siblings, 1 reply; 5+ messages in thread
From: Takashi Sakamoto @ 2025-06-14 11:34 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Some tasklets (softIRQs) are still used as bottom-halves to handle
events for 1394 OHCI AR/AT contexts. However, using softIRQs for IRQ
bottom halves is generally discouraged today.

This commit adds a per-fw_card workqueue to accommodate the behaviour
specified by the 1394 OHCI specification.

According to the 1394 OHCI specification, system memory pages are
reserved for each asynchronous DMA context. This allows concurrent
operation across contexts. In the 1394 OHCI PCI driver implementation,
the hardware generates IRQs either upon receiving asynchronous packets
from other nodes (incoming) or after completing transmission to them
(outgoing). These independent events can occur in the same transmission
cycle, therefore the max_active parameter for the workqueue is set to the
total number of AR/AT contexts (=4). The WQ_UNBOUND flag is used to
allow the work to be scheduled on any available core, since there is
little CPU cache affinity benefit for the data.

Each DMA context uses a circular descriptor list in system memory,
allowing deferred data processing in software as long as buffer overrun
are avoided. Since the overall operation is sleepable except for small
atomic regions, WQ_BH is not used. As the descriptors contain
timestamps, WQ_HIGHPRI is specified to support semi-real-time
processing.

The asynchronous context is also used by the SCSI over IEEE 1394
protocol implementation (sbp2), which can be part of memory reclaim paths.
Therefore, WQ_MEM_RECLAIM is required.

To allow uses to adjust CPU affinity according to workload, WQ_SYSFS is
specified so that workqueue attributes are exposed to user space.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-card.c | 40 +++++++++++++++++++++++++++---------
 include/linux/firewire.h     |  1 +
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 2b6ad47b6d57..df0bb5b96ddc 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -574,7 +574,6 @@ EXPORT_SYMBOL(fw_card_initialize);
 int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
 		unsigned int supported_isoc_contexts)
 {
-	struct workqueue_struct *isoc_wq;
 	int ret;
 
 	// This workqueue should be:
@@ -589,12 +588,29 @@ int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
 	//  * == WQ_SYSFS		Parameters are available via sysfs.
 	//  * max_active == n_it + n_ir	A hardIRQ could notify events for multiple isochronous
 	//				contexts if they are scheduled to the same cycle.
-	isoc_wq = alloc_workqueue("firewire-isoc-card%u",
-				  WQ_UNBOUND | WQ_FREEZABLE | WQ_HIGHPRI | WQ_SYSFS,
-				  supported_isoc_contexts, card->index);
-	if (!isoc_wq)
+	card->isoc_wq = alloc_workqueue("firewire-isoc-card%u",
+					WQ_UNBOUND | WQ_FREEZABLE | WQ_HIGHPRI | WQ_SYSFS,
+					supported_isoc_contexts, card->index);
+	if (!card->isoc_wq)
 		return -ENOMEM;
 
+	// This workqueue should be:
+	//  * != WQ_BH			Sleepable.
+	//  * == WQ_UNBOUND		Any core can process data for asynchronous context.
+	//  * == WQ_MEM_RECLAIM		Used for any backend of block device.
+	//  * == WQ_FREEZABLE		The target device would not be available when being freezed.
+	//  * == WQ_HIGHPRI		High priority to process semi-realtime timestamped data.
+	//  * == WQ_SYSFS		Parameters are available via sysfs.
+	//  * max_active == 4		A hardIRQ could notify events for a pair of requests and
+	//				response AR/AT contexts.
+	card->async_wq = alloc_workqueue("firewire-async-card%u",
+					 WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_FREEZABLE | WQ_HIGHPRI | WQ_SYSFS,
+					 4, card->index);
+	if (!card->async_wq) {
+		ret = -ENOMEM;
+		goto err_isoc;
+	}
+
 	card->max_receive = max_receive;
 	card->link_speed = link_speed;
 	card->guid = guid;
@@ -603,15 +619,17 @@ int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
 
 	generate_config_rom(card, tmp_config_rom);
 	ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
-	if (ret < 0) {
-		destroy_workqueue(isoc_wq);
-		return ret;
-	}
+	if (ret < 0)
+		goto err_async;
 
-	card->isoc_wq = isoc_wq;
 	list_add_tail(&card->link, &card_list);
 
 	return 0;
+err_async:
+	destroy_workqueue(card->async_wq);
+err_isoc:
+	destroy_workqueue(card->isoc_wq);
+	return ret;
 }
 EXPORT_SYMBOL(fw_card_add);
 
@@ -744,6 +762,7 @@ void fw_core_remove_card(struct fw_card *card)
 	dummy_driver.stop_iso		= card->driver->stop_iso;
 	card->driver = &dummy_driver;
 	drain_workqueue(card->isoc_wq);
+	drain_workqueue(card->async_wq);
 
 	scoped_guard(spinlock_irqsave, &card->lock)
 		fw_destroy_nodes(card);
@@ -753,6 +772,7 @@ void fw_core_remove_card(struct fw_card *card)
 	wait_for_completion(&card->done);
 
 	destroy_workqueue(card->isoc_wq);
+	destroy_workqueue(card->async_wq);
 
 	WARN_ON(!list_empty(&card->transaction_list));
 }
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index b632eec3ab52..c55b8e30e700 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -136,6 +136,7 @@ struct fw_card {
 	__be32 maint_utility_register;
 
 	struct workqueue_struct *isoc_wq;
+	struct workqueue_struct *async_wq;
 };
 
 static inline struct fw_card *fw_card_get(struct fw_card *card)
-- 
2.48.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] firewire: ohci: use workqueue to handle events of AR request/response contexts
  2025-06-14 11:34 [PATCH 0/3] firewire: ohci: use regular workqueue to handle 1394 OHCI AT/AR context events Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts Takashi Sakamoto
@ 2025-06-14 11:34 ` Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 3/3] firewire: ohci: use workqueue to handle events of AT " Takashi Sakamoto
  2 siblings, 0 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2025-06-14 11:34 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This commit adds a work item to handle events of 1394 OHCI AR
request/response contexts, and queues the item to the specific workqueue.
The call of struct fw_address_handler.address_callback() is done in the
workqueue when receiving any requests from the remove nodes. Additionally,
the call of struct fw_packet.callback() is done in the workqueue too when
receiving acknowledge to the asynchronous packet for the response
subaction of split transaction to the remote nodes.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c |  7 ++++---
 drivers/firewire/ohci.c             | 27 +++++++++++----------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 2bd5deb9054e..d28477d84697 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -557,9 +557,10 @@ const struct fw_address_region fw_unit_space_region =
  *
  * region->start, ->end, and handler->length have to be quadlet-aligned.
  *
- * 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.
+ * 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.
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 40313a3ec63e..68317b5a64a7 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -101,7 +101,7 @@ struct ar_context {
 	void *pointer;
 	unsigned int last_buffer_index;
 	u32 regs;
-	struct tasklet_struct tasklet;
+	struct work_struct work;
 };
 
 struct context;
@@ -1016,9 +1016,9 @@ static void ar_recycle_buffers(struct ar_context *ctx, unsigned int end_buffer)
 	}
 }
 
-static void ar_context_tasklet(unsigned long data)
+static void ohci_ar_context_work(struct work_struct *work)
 {
-	struct ar_context *ctx = (struct ar_context *)data;
+	struct ar_context *ctx = from_work(ctx, work, work);
 	unsigned int end_buffer_index, end_buffer_offset;
 	void *p, *end;
 
@@ -1026,23 +1026,19 @@ static void ar_context_tasklet(unsigned long data)
 	if (!p)
 		return;
 
-	end_buffer_index = ar_search_last_active_buffer(ctx,
-							&end_buffer_offset);
+	end_buffer_index = ar_search_last_active_buffer(ctx, &end_buffer_offset);
 	ar_sync_buffers_for_cpu(ctx, end_buffer_index, end_buffer_offset);
 	end = ctx->buffer + end_buffer_index * PAGE_SIZE + end_buffer_offset;
 
 	if (end_buffer_index < ar_first_buffer_index(ctx)) {
-		/*
-		 * The filled part of the overall buffer wraps around; handle
-		 * all packets up to the buffer end here.  If the last packet
-		 * wraps around, its tail will be visible after the buffer end
-		 * because the buffer start pages are mapped there again.
-		 */
+		// The filled part of the overall buffer wraps around; handle all packets up to the
+		// buffer end here.  If the last packet wraps around, its tail will be visible after
+		// the buffer end because the buffer start pages are mapped there again.
 		void *buffer_end = ctx->buffer + AR_BUFFERS * PAGE_SIZE;
 		p = handle_ar_packets(ctx, p, buffer_end);
 		if (p < buffer_end)
 			goto error;
-		/* adjust p to point back into the actual buffer */
+		// adjust p to point back into the actual buffer
 		p -= AR_BUFFERS * PAGE_SIZE;
 	}
 
@@ -1057,7 +1053,6 @@ static void ar_context_tasklet(unsigned long data)
 	ar_recycle_buffers(ctx, end_buffer_index);
 
 	return;
-
 error:
 	ctx->pointer = NULL;
 }
@@ -1073,7 +1068,7 @@ static int ar_context_init(struct ar_context *ctx, struct fw_ohci *ohci,
 
 	ctx->regs        = regs;
 	ctx->ohci        = ohci;
-	tasklet_init(&ctx->tasklet, ar_context_tasklet, (unsigned long)ctx);
+	INIT_WORK(&ctx->work, ohci_ar_context_work);
 
 	for (i = 0; i < AR_BUFFERS; i++) {
 		ctx->pages[i] = dma_alloc_pages(dev, PAGE_SIZE, &dma_addr,
@@ -2238,10 +2233,10 @@ static irqreturn_t irq_handler(int irq, void *data)
 	}
 
 	if (event & OHCI1394_RQPkt)
-		tasklet_schedule(&ohci->ar_request_ctx.tasklet);
+		queue_work(ohci->card.async_wq, &ohci->ar_request_ctx.work);
 
 	if (event & OHCI1394_RSPkt)
-		tasklet_schedule(&ohci->ar_response_ctx.tasklet);
+		queue_work(ohci->card.async_wq, &ohci->ar_response_ctx.work);
 
 	if (event & OHCI1394_reqTxComplete)
 		tasklet_schedule(&ohci->at_request_ctx.tasklet);
-- 
2.48.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] firewire: ohci: use workqueue to handle events of AT request/response contexts
  2025-06-14 11:34 [PATCH 0/3] firewire: ohci: use regular workqueue to handle 1394 OHCI AT/AR context events Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts Takashi Sakamoto
  2025-06-14 11:34 ` [PATCH 2/3] firewire: ohci: use workqueue to handle events of AR " Takashi Sakamoto
@ 2025-06-14 11:34 ` Takashi Sakamoto
  2 siblings, 0 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2025-06-14 11:34 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

This commit adds a work item to handle events of 1394 OHCI AT
request/response contexts, and queues the item to the specific
workqueue. The call of struct fw_packet.callbaqck() is done in the
workqueue when receiving acknowledgement to the asynchronous packet
transferred to remote node.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/net.c   |  4 ++--
 drivers/firewire/ohci.c  | 40 ++++++++++++++++++++++++----------------
 include/linux/firewire.h | 11 +++++++++--
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 1bf0e15c1540..6d6446713539 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1007,7 +1007,7 @@ static int fwnet_send_packet(struct fwnet_packet_task *ptask)
 
 		spin_lock_irqsave(&dev->lock, flags);
 
-		/* If the AT tasklet already ran, we may be last user. */
+		/* If the AT work item already ran, we may be last user. */
 		free = (ptask->outstanding_pkts == 0 && !ptask->enqueued);
 		if (!free)
 			ptask->enqueued = true;
@@ -1026,7 +1026,7 @@ static int fwnet_send_packet(struct fwnet_packet_task *ptask)
 
 	spin_lock_irqsave(&dev->lock, flags);
 
-	/* If the AT tasklet already ran, we may be last user. */
+	/* If the AT work item already ran, we may be last user. */
 	free = (ptask->outstanding_pkts == 0 && !ptask->enqueued);
 	if (!free)
 		ptask->enqueued = true;
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 68317b5a64a7..a81a876819d0 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -158,7 +158,7 @@ struct context {
 
 	descriptor_callback_t callback;
 
-	struct tasklet_struct tasklet;
+	struct work_struct work;
 };
 
 struct iso_context {
@@ -1176,9 +1176,9 @@ static void context_retire_descriptors(struct context *ctx)
 	}
 }
 
-static void context_tasklet(unsigned long data)
+static void ohci_at_context_work(struct work_struct *work)
 {
-	struct context *ctx = (struct context *) data;
+	struct context *ctx = from_work(ctx, work, work);
 
 	context_retire_descriptors(ctx);
 }
@@ -1243,7 +1243,6 @@ static int context_init(struct context *ctx, struct fw_ohci *ohci,
 	ctx->buffer_tail = list_entry(ctx->buffer_list.next,
 			struct descriptor_buffer, list);
 
-	tasklet_init(&ctx->tasklet, context_tasklet, (unsigned long)ctx);
 	ctx->callback = callback;
 
 	/*
@@ -1524,13 +1523,17 @@ static int at_context_queue_packet(struct context *ctx,
 
 static void at_context_flush(struct context *ctx)
 {
-	tasklet_disable(&ctx->tasklet);
+        // Avoid dead lock due to programming mistake.
+        if (WARN_ON_ONCE(current_work() == &ctx->work))
+                return;
+
+	disable_work_sync(&ctx->work);
 
-	ctx->flushing = true;
-	context_tasklet((unsigned long)ctx);
-	ctx->flushing = false;
+	WRITE_ONCE(ctx->flushing, true);
+	ohci_at_context_work(&ctx->work);
+	WRITE_ONCE(ctx->flushing, false);
 
-	tasklet_enable(&ctx->tasklet);
+	enable_work(&ctx->work);
 }
 
 static int handle_at_packet(struct context *context,
@@ -1542,7 +1545,7 @@ static int handle_at_packet(struct context *context,
 	struct fw_ohci *ohci = context->ohci;
 	int evt;
 
-	if (last->transfer_status == 0 && !context->flushing)
+	if (last->transfer_status == 0 && !READ_ONCE(context->flushing))
 		/* This descriptor isn't done yet, stop iteration. */
 		return 0;
 
@@ -1576,7 +1579,7 @@ static int handle_at_packet(struct context *context,
 		break;
 
 	case OHCI1394_evt_missing_ack:
-		if (context->flushing)
+		if (READ_ONCE(context->flushing))
 			packet->ack = RCODE_GENERATION;
 		else {
 			/*
@@ -1598,7 +1601,7 @@ static int handle_at_packet(struct context *context,
 		break;
 
 	case OHCI1394_evt_no_status:
-		if (context->flushing) {
+		if (READ_ONCE(context->flushing)) {
 			packet->ack = RCODE_GENERATION;
 			break;
 		}
@@ -2239,10 +2242,10 @@ static irqreturn_t irq_handler(int irq, void *data)
 		queue_work(ohci->card.async_wq, &ohci->ar_response_ctx.work);
 
 	if (event & OHCI1394_reqTxComplete)
-		tasklet_schedule(&ohci->at_request_ctx.tasklet);
+		queue_work(ohci->card.async_wq, &ohci->at_request_ctx.work);
 
 	if (event & OHCI1394_respTxComplete)
-		tasklet_schedule(&ohci->at_response_ctx.tasklet);
+		queue_work(ohci->card.async_wq, &ohci->at_response_ctx.work);
 
 	if (event & OHCI1394_isochRx) {
 		iso_event = reg_read(ohci, OHCI1394_IsoRecvIntEventClear);
@@ -2684,7 +2687,10 @@ static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet)
 	struct driver_data *driver_data = packet->driver_data;
 	int ret = -ENOENT;
 
-	tasklet_disable_in_atomic(&ctx->tasklet);
+        // Avoid dead lock due to programming mistake.
+        if (WARN_ON_ONCE(current_work() == &ctx->work))
+                return 0;
+	disable_work_sync(&ctx->work);
 
 	if (packet->ack != 0)
 		goto out;
@@ -2703,7 +2709,7 @@ static int ohci_cancel_packet(struct fw_card *card, struct fw_packet *packet)
 	packet->callback(packet, &ohci->card, packet->ack);
 	ret = 0;
  out:
-	tasklet_enable(&ctx->tasklet);
+	enable_work(&ctx->work);
 
 	return ret;
 }
@@ -3765,11 +3771,13 @@ static int pci_probe(struct pci_dev *dev,
 			   OHCI1394_AsReqTrContextControlSet, handle_at_packet);
 	if (err < 0)
 		return err;
+	INIT_WORK(&ohci->at_request_ctx.work, ohci_at_context_work);
 
 	err = context_init(&ohci->at_response_ctx, ohci,
 			   OHCI1394_AsRspTrContextControlSet, handle_at_packet);
 	if (err < 0)
 		return err;
+	INIT_WORK(&ohci->at_response_ctx.work, ohci_at_context_work);
 
 	reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0);
 	ohci->ir_context_channels = ~0ULL;
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index c55b8e30e700..cceb70415ed2 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -308,8 +308,7 @@ struct fw_packet {
 	 * For successful transmission, the status code is the ack received
 	 * from the destination.  Otherwise it is one of the juju-specific
 	 * rcodes:  RCODE_SEND_ERROR, _CANCELLED, _BUSY, _GENERATION, _NO_ACK.
-	 * The callback can be called from tasklet context and thus
-	 * must never block.
+	 * The callback can be called from workqueue and thus must never block.
 	 */
 	fw_packet_callback_t callback;
 	int ack;
@@ -382,6 +381,10 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
  *
  * A variation of __fw_send_request() to generate callback for response subaction without time
  * stamp.
+ *
+ * The callback is invoked in the workqueue context in most cases. However, if an error is detected
+ * before queueing or the destination address refers to the local node, it is invoked in the
+ * current context instead.
  */
 static inline void fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode,
 				   int destination_id, int generation, int speed,
@@ -411,6 +414,10 @@ static inline void fw_send_request(struct fw_card *card, struct fw_transaction *
  * @callback_data:	data to be passed to the transaction completion callback
  *
  * A variation of __fw_send_request() to generate callback for response subaction with time stamp.
+ *
+ * The callback is invoked in the workqueue context in most cases. However, if an error is detected
+ * before queueing or the destination address refers to the local node, it is invoked in the current
+ * context instead.
  */
 static inline void fw_send_request_with_tstamp(struct fw_card *card, struct fw_transaction *t,
 	int tcode, int destination_id, int generation, int speed, unsigned long long offset,
-- 
2.48.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts
  2025-06-14 11:34 ` [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts Takashi Sakamoto
@ 2025-06-15  4:40   ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-06-15  4:40 UTC (permalink / raw)
  To: Takashi Sakamoto, linux1394-devel; +Cc: llvm, oe-kbuild-all, linux-kernel

Hi Takashi,

kernel test robot noticed the following build errors:

[auto build test ERROR on ieee1394-linux1394/for-next]
[also build test ERROR on linus/master v6.16-rc1 next-20250613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Takashi-Sakamoto/firewire-core-allocate-workqueue-for-AR-AT-request-response-contexts/20250614-194424
base:   https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git for-next
patch link:    https://lore.kernel.org/r/20250614113449.388758-2-o-takashi%40sakamocchi.jp
patch subject: [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts
config: x86_64-buildonly-randconfig-004-20250615 (https://download.01.org/0day-ci/archive/20250615/202506151242.vncWEzlc-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250615/202506151242.vncWEzlc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506151242.vncWEzlc-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/firewire/core-card.c:611:3: error: cannot jump from this goto statement to its label
     611 |                 goto err_isoc;
         |                 ^
   drivers/firewire/core-card.c:618:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
     618 |         guard(mutex)(&card_mutex);
         |         ^
   include/linux/cleanup.h:338:15: note: expanded from macro 'guard'
     338 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:71:1: note: expanded from here
      71 | __UNIQUE_ID_guard795
         | ^
   1 error generated.


vim +611 drivers/firewire/core-card.c

   573	
   574	int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
   575			unsigned int supported_isoc_contexts)
   576	{
   577		int ret;
   578	
   579		// This workqueue should be:
   580		//  * != WQ_BH			Sleepable.
   581		//  * == WQ_UNBOUND		Any core can process data for isoc context. The
   582		//				implementation of unit protocol could consumes the core
   583		//				longer somehow.
   584		//  * != WQ_MEM_RECLAIM		Not used for any backend of block device.
   585		//  * == WQ_FREEZABLE		Isochronous communication is at regular interval in real
   586		//				time, thus should be drained if possible at freeze phase.
   587		//  * == WQ_HIGHPRI		High priority to process semi-realtime timestamped data.
   588		//  * == WQ_SYSFS		Parameters are available via sysfs.
   589		//  * max_active == n_it + n_ir	A hardIRQ could notify events for multiple isochronous
   590		//				contexts if they are scheduled to the same cycle.
   591		card->isoc_wq = alloc_workqueue("firewire-isoc-card%u",
   592						WQ_UNBOUND | WQ_FREEZABLE | WQ_HIGHPRI | WQ_SYSFS,
   593						supported_isoc_contexts, card->index);
   594		if (!card->isoc_wq)
   595			return -ENOMEM;
   596	
   597		// This workqueue should be:
   598		//  * != WQ_BH			Sleepable.
   599		//  * == WQ_UNBOUND		Any core can process data for asynchronous context.
   600		//  * == WQ_MEM_RECLAIM		Used for any backend of block device.
   601		//  * == WQ_FREEZABLE		The target device would not be available when being freezed.
   602		//  * == WQ_HIGHPRI		High priority to process semi-realtime timestamped data.
   603		//  * == WQ_SYSFS		Parameters are available via sysfs.
   604		//  * max_active == 4		A hardIRQ could notify events for a pair of requests and
   605		//				response AR/AT contexts.
   606		card->async_wq = alloc_workqueue("firewire-async-card%u",
   607						 WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_FREEZABLE | WQ_HIGHPRI | WQ_SYSFS,
   608						 4, card->index);
   609		if (!card->async_wq) {
   610			ret = -ENOMEM;
 > 611			goto err_isoc;
   612		}
   613	
   614		card->max_receive = max_receive;
   615		card->link_speed = link_speed;
   616		card->guid = guid;
   617	
   618		guard(mutex)(&card_mutex);
   619	
   620		generate_config_rom(card, tmp_config_rom);
   621		ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
   622		if (ret < 0)
   623			goto err_async;
   624	
   625		list_add_tail(&card->link, &card_list);
   626	
   627		return 0;
   628	err_async:
   629		destroy_workqueue(card->async_wq);
   630	err_isoc:
   631		destroy_workqueue(card->isoc_wq);
   632		return ret;
   633	}
   634	EXPORT_SYMBOL(fw_card_add);
   635	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-15  4:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-14 11:34 [PATCH 0/3] firewire: ohci: use regular workqueue to handle 1394 OHCI AT/AR context events Takashi Sakamoto
2025-06-14 11:34 ` [PATCH 1/3] firewire: core: allocate workqueue for AR/AT request/response contexts Takashi Sakamoto
2025-06-15  4:40   ` kernel test robot
2025-06-14 11:34 ` [PATCH 2/3] firewire: ohci: use workqueue to handle events of AR " Takashi Sakamoto
2025-06-14 11:34 ` [PATCH 3/3] firewire: ohci: use workqueue to handle events of AT " Takashi Sakamoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®