[PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Zhang Lixu]

During suspend/resume tests with S2IDLE, some ISH functional failures were
observed because of delay in executing ISH resume handler. Here
schedule_work() is used from resume handler to do actual work.
schedule_work() uses system_wq, which is a per CPU work queue. Although
the queuing is not bound to a CPU, but it prefers local CPU of the caller,
unless prohibited.

Users of this work queue are not supposed to queue long running work.
But in practice, there are scenarios where long running work items are
queued on other unbound workqueues, occupying the CPU. As a result, the
ISH resume handler may not get a chance to execute in a timely manner.

In one scenario, one of the ish_resume_handler() executions was delayed
nearly 1 second because another work item on an unbound workqueue occupied
the same CPU. This delay causes ISH functionality failures.

A similar issue was previously observed where the ISH HID driver timed out
while getting the HID descriptor during S4 resume in the recovery kernel,
likely caused by the same workqueue contention problem.

Create dedicated unbound workqueues for all ISH operations to allow work
items to execute on any available CPU, eliminating CPU-specific bottlenecks
and improving resume reliability under varying system loads. Also ISH has
three different components, a bus driver which implements ISH protocols, a
PCI interface layer and HID interface. Use one dedicated work queue for all
of them.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c          | 21 +++++++++++++++++++-
 drivers/hid/intel-ish-hid/ipc/pci-ish.c      |  2 +-
 drivers/hid/intel-ish-hid/ishtp-hid-client.c |  4 ++--
 drivers/hid/intel-ish-hid/ishtp/bus.c        | 18 ++++++++++++++++-
 drivers/hid/intel-ish-hid/ishtp/hbm.c        |  4 ++--
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h  |  3 +++
 include/linux/intel-ish-client-if.h          |  2 ++
 7 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 3ddaa2cd39d5..9958f2968c4f 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -628,7 +628,7 @@ static void	recv_ipc(struct ishtp_device *dev, uint32_t doorbell_val)
 		if (!ishtp_dev) {
 			ishtp_dev = dev;
 		}
-		schedule_work(&fw_reset_work);
+		queue_work(dev->unbound_wq, &fw_reset_work);
 		break;
 
 	case MNG_RESET_NOTIFY_ACK:
@@ -933,6 +933,21 @@ static const struct ishtp_hw_ops ish_hw_ops = {
 	.dma_no_cache_snooping = _dma_no_cache_snooping
 };
 
+static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev)
+{
+	struct workqueue_struct *wq;
+
+	wq = alloc_workqueue("ishtp_unbound_%d", WQ_UNBOUND, 0, dev->id);
+	if (!wq)
+		return NULL;
+
+	if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
+				     wq))
+		return NULL;
+
+	return wq;
+}
+
 /**
  * ish_dev_init() -Initialize ISH devoce
  * @pdev: PCI device
@@ -953,6 +968,10 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
 	if (!dev)
 		return NULL;
 
+	dev->unbound_wq = devm_ishtp_alloc_workqueue(&pdev->dev);
+	if (!dev->unbound_wq)
+		return NULL;
+
 	dev->devc = &pdev->dev;
 	ishtp_device_init(dev);
 
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 9d150ce234f2..b748ac6fbfdc 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -384,7 +384,7 @@ static int __maybe_unused ish_resume(struct device *device)
 	ish_resume_device = device;
 	dev->resume_flag = 1;
 
-	schedule_work(&resume_work);
+	queue_work(dev->unbound_wq, &resume_work);
 
 	return 0;
 }
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index d8c3c54a8c0f..f61add862b6b 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -860,7 +860,7 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
 
-	schedule_work(&client_data->work);
+	queue_work(ishtp_get_workqueue(cl_device), &client_data->work);
 
 	return 0;
 }
@@ -902,7 +902,7 @@ static int hid_ishtp_cl_resume(struct device *device)
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
-	schedule_work(&client_data->resume_work);
+	queue_work(ishtp_get_workqueue(cl_device), &client_data->resume_work);
 	return 0;
 }
 
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 93a0432e7058..c6ce37244e49 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -541,7 +541,7 @@ void ishtp_cl_bus_rx_event(struct ishtp_cl_device *device)
 		return;
 
 	if (device->event_cb)
-		schedule_work(&device->event_work);
+		queue_work(device->ishtp_dev->unbound_wq, &device->event_work);
 }
 
 /**
@@ -876,6 +876,22 @@ struct device *ishtp_get_pci_device(struct ishtp_cl_device *device)
 }
 EXPORT_SYMBOL(ishtp_get_pci_device);
 
+/**
+ * ishtp_get_workqueue - Retrieve the workqueue associated with an ISHTP device
+ * @cl_device: Pointer to the ISHTP client device structure
+ *
+ * Returns the workqueue_struct pointer (unbound_wq) associated with the given
+ * ISHTP client device. This workqueue is typically used for scheduling work
+ * related to the device.
+ *
+ * Return: Pointer to struct workqueue_struct.
+ */
+struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device)
+{
+	return cl_device->ishtp_dev->unbound_wq;
+}
+EXPORT_SYMBOL(ishtp_get_workqueue);
+
 /**
  * ishtp_trace_callback() - Return trace callback
  * @cl_device: ISH-TP client device instance
diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.c b/drivers/hid/intel-ish-hid/ishtp/hbm.c
index 8ee5467127d8..97c4fcd9e3c6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/hbm.c
+++ b/drivers/hid/intel-ish-hid/ishtp/hbm.c
@@ -573,7 +573,7 @@ void ishtp_hbm_dispatch(struct ishtp_device *dev,
 
 		/* Start firmware loading process if it has loader capability */
 		if (version_res->host_version_supported & ISHTP_SUPPORT_CAP_LOADER)
-			schedule_work(&dev->work_fw_loader);
+			queue_work(dev->unbound_wq, &dev->work_fw_loader);
 
 		dev->version.major_version = HBM_MAJOR_VERSION;
 		dev->version.minor_version = HBM_MINOR_VERSION;
@@ -864,7 +864,7 @@ void	recv_hbm(struct ishtp_device *dev, struct ishtp_msg_hdr *ishtp_hdr)
 	dev->rd_msg_fifo_tail = (dev->rd_msg_fifo_tail + IPC_PAYLOAD_SIZE) %
 		(RD_INT_FIFO_SIZE * IPC_PAYLOAD_SIZE);
 	spin_unlock_irqrestore(&dev->rd_msg_spinlock, flags);
-	schedule_work(&dev->bh_hbm_work);
+	queue_work(dev->unbound_wq, &dev->bh_hbm_work);
 eoi:
 	return;
 }
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index 23db97ecf21c..4b0596eadf1c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -175,6 +175,9 @@ struct ishtp_device {
 	struct hbm_version version;
 	int transfer_path; /* Choice of transfer path: IPC or DMA */
 
+	/* Alloc a dedicated unbound workqueue for ishtp device */
+	struct workqueue_struct *unbound_wq;
+
 	/* work structure for scheduling firmware loading tasks */
 	struct work_struct work_fw_loader;
 	/* waitq for waiting for command response from the firmware loader */
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index dfbf7d9d7bb5..b235fd84f478 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -87,6 +87,8 @@ bool ishtp_wait_resume(struct ishtp_device *dev);
 ishtp_print_log ishtp_trace_callback(struct ishtp_cl_device *cl_device);
 /* Get device pointer of PCI device for DMA acces */
 struct device *ishtp_get_pci_device(struct ishtp_cl_device *cl_device);
+/* Get the ISHTP workqueue */
+struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device);
 
 struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
 void ishtp_cl_free(struct ishtp_cl *cl);

base-commit: 0b2f041c47acb45db82b4e847af6e17eb66cd32d
-- 
2.43.0

Re: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Jiri Kosina]

On Fri, 10 Oct 2025, Zhang Lixu wrote:

> During suspend/resume tests with S2IDLE, some ISH functional failures were
> observed because of delay in executing ISH resume handler. Here
> schedule_work() is used from resume handler to do actual work.
> schedule_work() uses system_wq, which is a per CPU work queue. Although
> the queuing is not bound to a CPU, but it prefers local CPU of the caller,
> unless prohibited.
> 
> Users of this work queue are not supposed to queue long running work.
> But in practice, there are scenarios where long running work items are
> queued on other unbound workqueues, occupying the CPU. As a result, the
> ISH resume handler may not get a chance to execute in a timely manner.
> 
> In one scenario, one of the ish_resume_handler() executions was delayed
> nearly 1 second because another work item on an unbound workqueue occupied
> the same CPU. This delay causes ISH functionality failures.
> 
> A similar issue was previously observed where the ISH HID driver timed out
> while getting the HID descriptor during S4 resume in the recovery kernel,
> likely caused by the same workqueue contention problem.
> 
> Create dedicated unbound workqueues for all ISH operations to allow work
> items to execute on any available CPU, eliminating CPU-specific bottlenecks
> and improving resume reliability under varying system loads. Also ISH has
> three different components, a bus driver which implements ISH protocols, a
> PCI interface layer and HID interface. Use one dedicated work queue for all
> of them.
> 
> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>

How serious / widespread / easy to trigger is the issue? My reading of 
this is that it should be merged still in this cycle (i.e. for 6.18), but 
I'd like to have that confirmed.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[srinivas pandruvada]

On Tue, 2025-10-14 at 15:19 +0200, Jiri Kosina wrote:
> On Fri, 10 Oct 2025, Zhang Lixu wrote:
> 
> > During suspend/resume tests with S2IDLE, some ISH functional
> > failures were
> > observed because of delay in executing ISH resume handler. Here
> > schedule_work() is used from resume handler to do actual work.
> > schedule_work() uses system_wq, which is a per CPU work queue.
> > Although
> > the queuing is not bound to a CPU, but it prefers local CPU of the
> > caller,
> > unless prohibited.
> > 
> > Users of this work queue are not supposed to queue long running
> > work.
> > But in practice, there are scenarios where long running work items
> > are
> > queued on other unbound workqueues, occupying the CPU. As a result,
> > the
> > ISH resume handler may not get a chance to execute in a timely
> > manner.
> > 
> > In one scenario, one of the ish_resume_handler() executions was
> > delayed
> > nearly 1 second because another work item on an unbound workqueue
> > occupied
> > the same CPU. This delay causes ISH functionality failures.
> > 
> > A similar issue was previously observed where the ISH HID driver
> > timed out
> > while getting the HID descriptor during S4 resume in the recovery
> > kernel,
> > likely caused by the same workqueue contention problem.
> > 
> > Create dedicated unbound workqueues for all ISH operations to allow
> > work
> > items to execute on any available CPU, eliminating CPU-specific
> > bottlenecks
> > and improving resume reliability under varying system loads. Also
> > ISH has
> > three different components, a bus driver which implements ISH
> > protocols, a
> > PCI interface layer and HID interface. Use one dedicated work queue
> > for all
> > of them.
> > 
> > Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
> 
> How serious / widespread / easy to trigger is the issue? My reading
> of 
> this is that it should be merged still in this cycle (i.e. for 6.18),
> but 
> I'd like to have that confirmed.

I don't think we notice this issue in our regular cyclic tests of 100s
of cycles. So I think this can wait. Lixu, what do you think?

Thanks,
Srinivas


> 
> Thanks,

RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Zhang, Lixu]

>-----Original Message-----
>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Sent: Wednesday, October 15, 2025 12:27 AM
>To: Jiri Kosina <jikos@kernel.org>; Zhang, Lixu <lixu.zhang@intel.com>
>Cc: linux-input@vger.kernel.org; benjamin.tissoires@redhat.com; Wang, Selina
><selina.wang@intel.com>
>Subject: Re: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to
>prevent resume blocking
>
>On Tue, 2025-10-14 at 15:19 +0200, Jiri Kosina wrote:
>> On Fri, 10 Oct 2025, Zhang Lixu wrote:
>>
>> > During suspend/resume tests with S2IDLE, some ISH functional
>> > failures were observed because of delay in executing ISH resume
>> > handler. Here
>> > schedule_work() is used from resume handler to do actual work.
>> > schedule_work() uses system_wq, which is a per CPU work queue.
>> > Although
>> > the queuing is not bound to a CPU, but it prefers local CPU of the
>> > caller, unless prohibited.
>> >
>> > Users of this work queue are not supposed to queue long running
>> > work.
>> > But in practice, there are scenarios where long running work items
>> > are queued on other unbound workqueues, occupying the CPU. As a
>> > result, the ISH resume handler may not get a chance to execute in a
>> > timely manner.
>> >
>> > In one scenario, one of the ish_resume_handler() executions was
>> > delayed nearly 1 second because another work item on an unbound
>> > workqueue occupied the same CPU. This delay causes ISH functionality
>> > failures.
>> >
>> > A similar issue was previously observed where the ISH HID driver
>> > timed out while getting the HID descriptor during S4 resume in the
>> > recovery kernel, likely caused by the same workqueue contention
>> > problem.
>> >
>> > Create dedicated unbound workqueues for all ISH operations to allow
>> > work items to execute on any available CPU, eliminating CPU-specific
>> > bottlenecks and improving resume reliability under varying system
>> > loads. Also ISH has three different components, a bus driver which
>> > implements ISH protocols, a PCI interface layer and HID interface.
>> > Use one dedicated work queue for all of them.
>> >
>> > Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
>>
>> How serious / widespread / easy to trigger is the issue? My reading of
>> this is that it should be merged still in this cycle (i.e. for 6.18),
>> but I'd like to have that confirmed.
>
>I don't think we notice this issue in our regular cyclic tests of 100s of cycles. So
>I think this can wait. Lixu, what do you think?

I personally prefer to have this patch merged before the patch series [PATCH 0/6] HID: intel-ish-hid: Various power management, since a merge conflict exists between this patch and that patch series. If this patch is merged first, it would be more convenient for others to backport this patch to other kernel versions.
BTW, in this case, the patch series [PATCH 0/6] HID: intel-ish-hid: Various power management would need to be rebased. Do I need to send a v2 patch series for the rebase?

Thanks,
Lixu

>
>Thanks,
>Srinivas
>
>
>>
>> Thanks,

RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Jiri Kosina]

On Wed, 15 Oct 2025, Zhang, Lixu wrote:

> I personally prefer to have this patch merged before the patch series 
> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a merge 
> conflict exists between this patch and that patch series. If this patch 
> is merged first, it would be more convenient for others to backport this 
> patch to other kernel versions. BTW, in this case, the patch series 
> [PATCH 0/6] HID: intel-ish-hid: Various power management would need to 
> be rebased. Do I need to send a v2 patch series for the rebase?

We can definitely do that. The question is whether this one should go in 
right now for 6.18 still, or we queue both for 6.19.

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Zhang, Lixu]

>-----Original Message-----
>From: Jiri Kosina <jikos@kernel.org>
>Sent: Wednesday, October 15, 2025 10:08 PM
>To: Zhang, Lixu <lixu.zhang@intel.com>
>Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; linux-
>input@vger.kernel.org; benjamin.tissoires@redhat.com; Wang, Selina
><selina.wang@intel.com>
>Subject: RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to
>prevent resume blocking
>
>On Wed, 15 Oct 2025, Zhang, Lixu wrote:
>
>> I personally prefer to have this patch merged before the patch series
>> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a
>> merge conflict exists between this patch and that patch series. If
>> this patch is merged first, it would be more convenient for others to
>> backport this patch to other kernel versions. BTW, in this case, the
>> patch series [PATCH 0/6] HID: intel-ish-hid: Various power management
>> would need to be rebased. Do I need to send a v2 patch series for the
>rebase?
>
>We can definitely do that. The question is whether this one should go in right
>now for 6.18 still, or we queue both for 6.19.
You can queue both for 6.19. Thanks.

Best regards,
Lixu

>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs

RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Jiri Kosina]

On Thu, 16 Oct 2025, Zhang, Lixu wrote:

> >> I personally prefer to have this patch merged before the patch series
> >> [PATCH 0/6] HID: intel-ish-hid: Various power management, since a
> >> merge conflict exists between this patch and that patch series. If
> >> this patch is merged first, it would be more convenient for others to
> >> backport this patch to other kernel versions. BTW, in this case, the
> >> patch series [PATCH 0/6] HID: intel-ish-hid: Various power management
> >> would need to be rebased. Do I need to send a v2 patch series for the
> >rebase?
> >
> >We can definitely do that. The question is whether this one should go in right
> >now for 6.18 still, or we queue both for 6.19.
> You can queue both for 6.19. Thanks.

Could you then please send me the power management series rebased on top 
of it, and I'll create a -v2 topic branch with all of it together.

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Zhang, Lixu]

>-----Original Message-----
>From: Jiri Kosina <jikos@kernel.org>
>Sent: Thursday, October 16, 2025 4:28 PM
>To: Zhang, Lixu <lixu.zhang@intel.com>
>Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; linux-
>input@vger.kernel.org; benjamin.tissoires@redhat.com; Wang, Selina
><selina.wang@intel.com>
>Subject: RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to
>prevent resume blocking
>
>On Thu, 16 Oct 2025, Zhang, Lixu wrote:
>
>> >
>> >We can definitely do that. The question is whether this one should go
>> >in right now for 6.18 still, or we queue both for 6.19.
>> You can queue both for 6.19. Thanks.
>
>Could you then please send me the power management series rebased on top
>of it, and I'll create a -v2 topic branch with all of it together.

Sure. By the way, I have made a small fix change for power management series.
I'll squash it into v2.

Thanks,
Lixu

>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs

Re: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking

[Jiri Kosina]

On Fri, 10 Oct 2025, Zhang Lixu wrote:

> During suspend/resume tests with S2IDLE, some ISH functional failures were
> observed because of delay in executing ISH resume handler. Here
> schedule_work() is used from resume handler to do actual work.
> schedule_work() uses system_wq, which is a per CPU work queue. Although
> the queuing is not bound to a CPU, but it prefers local CPU of the caller,
> unless prohibited.
> 
> Users of this work queue are not supposed to queue long running work.
> But in practice, there are scenarios where long running work items are
> queued on other unbound workqueues, occupying the CPU. As a result, the
> ISH resume handler may not get a chance to execute in a timely manner.
> 
> In one scenario, one of the ish_resume_handler() executions was delayed
> nearly 1 second because another work item on an unbound workqueue occupied
> the same CPU. This delay causes ISH functionality failures.
> 
> A similar issue was previously observed where the ISH HID driver timed out
> while getting the HID descriptor during S4 resume in the recovery kernel,
> likely caused by the same workqueue contention problem.
> 
> Create dedicated unbound workqueues for all ISH operations to allow work
> items to execute on any available CPU, eliminating CPU-specific bottlenecks
> and improving resume reliability under varying system loads. Also ISH has
> three different components, a bus driver which implements ISH protocols, a
> PCI interface layer and HID interface. Use one dedicated work queue for all
> of them.
> 
> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>

Applied to hid.git#for-6.19/intel-ish-v2, thanks.

-- 
Jiri Kosina
SUSE Labs