Skip to content

Commit 3d72fad

Browse files
lixuzhagregkh
authored andcommitted
HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
commit 0d30dae upstream. 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> Signed-off-by: Jiri Kosina <jkosina@suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 9f0a7ab commit 3d72fad

7 files changed

Lines changed: 47 additions & 7 deletions

File tree

drivers/hid/intel-ish-hid/ipc/ipc.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ static void recv_ipc(struct ishtp_device *dev, uint32_t doorbell_val)
628628
if (!ishtp_dev) {
629629
ishtp_dev = dev;
630630
}
631-
schedule_work(&fw_reset_work);
631+
queue_work(dev->unbound_wq, &fw_reset_work);
632632
break;
633633

634634
case MNG_RESET_NOTIFY_ACK:
@@ -933,6 +933,21 @@ static const struct ishtp_hw_ops ish_hw_ops = {
933933
.dma_no_cache_snooping = _dma_no_cache_snooping
934934
};
935935

936+
static struct workqueue_struct *devm_ishtp_alloc_workqueue(struct device *dev)
937+
{
938+
struct workqueue_struct *wq;
939+
940+
wq = alloc_workqueue("ishtp_unbound_%d", WQ_UNBOUND, 0, dev->id);
941+
if (!wq)
942+
return NULL;
943+
944+
if (devm_add_action_or_reset(dev, (void (*)(void *))destroy_workqueue,
945+
wq))
946+
return NULL;
947+
948+
return wq;
949+
}
950+
936951
/**
937952
* ish_dev_init() -Initialize ISH devoce
938953
* @pdev: PCI device
@@ -953,6 +968,10 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
953968
if (!dev)
954969
return NULL;
955970

971+
dev->unbound_wq = devm_ishtp_alloc_workqueue(&pdev->dev);
972+
if (!dev->unbound_wq)
973+
return NULL;
974+
956975
dev->devc = &pdev->dev;
957976
ishtp_device_init(dev);
958977

drivers/hid/intel-ish-hid/ipc/pci-ish.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ static int __maybe_unused ish_resume(struct device *device)
384384
ish_resume_device = device;
385385
dev->resume_flag = 1;
386386

387-
schedule_work(&resume_work);
387+
queue_work(dev->unbound_wq, &resume_work);
388388

389389
return 0;
390390
}

drivers/hid/intel-ish-hid/ishtp-hid-client.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
860860
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
861861
hid_ishtp_cl);
862862

863-
schedule_work(&client_data->work);
863+
queue_work(ishtp_get_workqueue(cl_device), &client_data->work);
864864

865865
return 0;
866866
}
@@ -902,7 +902,7 @@ static int hid_ishtp_cl_resume(struct device *device)
902902

903903
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
904904
hid_ishtp_cl);
905-
schedule_work(&client_data->resume_work);
905+
queue_work(ishtp_get_workqueue(cl_device), &client_data->resume_work);
906906
return 0;
907907
}
908908

drivers/hid/intel-ish-hid/ishtp/bus.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ void ishtp_cl_bus_rx_event(struct ishtp_cl_device *device)
541541
return;
542542

543543
if (device->event_cb)
544-
schedule_work(&device->event_work);
544+
queue_work(device->ishtp_dev->unbound_wq, &device->event_work);
545545
}
546546

547547
/**
@@ -876,6 +876,22 @@ struct device *ishtp_get_pci_device(struct ishtp_cl_device *device)
876876
}
877877
EXPORT_SYMBOL(ishtp_get_pci_device);
878878

879+
/**
880+
* ishtp_get_workqueue - Retrieve the workqueue associated with an ISHTP device
881+
* @cl_device: Pointer to the ISHTP client device structure
882+
*
883+
* Returns the workqueue_struct pointer (unbound_wq) associated with the given
884+
* ISHTP client device. This workqueue is typically used for scheduling work
885+
* related to the device.
886+
*
887+
* Return: Pointer to struct workqueue_struct.
888+
*/
889+
struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device)
890+
{
891+
return cl_device->ishtp_dev->unbound_wq;
892+
}
893+
EXPORT_SYMBOL(ishtp_get_workqueue);
894+
879895
/**
880896
* ishtp_trace_callback() - Return trace callback
881897
* @cl_device: ISH-TP client device instance

drivers/hid/intel-ish-hid/ishtp/hbm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ void ishtp_hbm_dispatch(struct ishtp_device *dev,
573573

574574
/* Start firmware loading process if it has loader capability */
575575
if (version_res->host_version_supported & ISHTP_SUPPORT_CAP_LOADER)
576-
schedule_work(&dev->work_fw_loader);
576+
queue_work(dev->unbound_wq, &dev->work_fw_loader);
577577

578578
dev->version.major_version = HBM_MAJOR_VERSION;
579579
dev->version.minor_version = HBM_MINOR_VERSION;
@@ -864,7 +864,7 @@ void recv_hbm(struct ishtp_device *dev, struct ishtp_msg_hdr *ishtp_hdr)
864864
dev->rd_msg_fifo_tail = (dev->rd_msg_fifo_tail + IPC_PAYLOAD_SIZE) %
865865
(RD_INT_FIFO_SIZE * IPC_PAYLOAD_SIZE);
866866
spin_unlock_irqrestore(&dev->rd_msg_spinlock, flags);
867-
schedule_work(&dev->bh_hbm_work);
867+
queue_work(dev->unbound_wq, &dev->bh_hbm_work);
868868
eoi:
869869
return;
870870
}

drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,9 @@ struct ishtp_device {
175175
struct hbm_version version;
176176
int transfer_path; /* Choice of transfer path: IPC or DMA */
177177

178+
/* Alloc a dedicated unbound workqueue for ishtp device */
179+
struct workqueue_struct *unbound_wq;
180+
178181
/* work structure for scheduling firmware loading tasks */
179182
struct work_struct work_fw_loader;
180183
/* waitq for waiting for command response from the firmware loader */

include/linux/intel-ish-client-if.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ bool ishtp_wait_resume(struct ishtp_device *dev);
8787
ishtp_print_log ishtp_trace_callback(struct ishtp_cl_device *cl_device);
8888
/* Get device pointer of PCI device for DMA acces */
8989
struct device *ishtp_get_pci_device(struct ishtp_cl_device *cl_device);
90+
/* Get the ISHTP workqueue */
91+
struct workqueue_struct *ishtp_get_workqueue(struct ishtp_cl_device *cl_device);
9092

9193
struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
9294
void ishtp_cl_free(struct ishtp_cl *cl);

0 commit comments

Comments
 (0)