[Intel-wired-lan] [[PATCH v2 iwl-next] v2 3/4] idpf: convert workqueues to unbound
Linga, Pavan Kumar
pavan.kumar.linga at intel.com
Thu Aug 29 16:02:04 UTC 2024
On 8/26/2024 11:10 AM, Manoj Vishwanathan wrote:
> From: Marco Leogrande <leogrande at google.com>
>
> When a workqueue is created with `WQ_UNBOUND`, its work items are
> served by special worker-pools, whose host workers are not bound to
> any specific CPU. In the default configuration (i.e. when
> `queue_delayed_work` and friends do not specify which CPU to run the
> work item on), `WQ_UNBOUND` allows the work item to be executed on any
> CPU in the same node of the CPU it was enqueued on. While this
> solution potentially sacrifices locality, it avoids contention with
> other processes that might dominate the CPU time of the processor the
> work item was scheduled on.
>
> This is not just a theoretical problem: in a praticular scenario > misconfigured process was hogging most of the time from CPU0, leaving
> less than 0.5% of its CPU time to the kworker. The IDPF workqueues
> that were using the kworker on CPU0 suffered large completion delays
> as a result, causing performance degradation, timeouts and eventual
> system crash.
>
> Tested:
>
> * I have also run a manual test to gauge the performance
> improvement. The test consists of an antagonist process
> (`./stress --cpu 2`) consuming as much of CPU 0 as possible. This
> process is run under `taskset 01` to bind it to CPU0, and its
> priority is changed with `chrt -pQ 9900 10000 ${pid}` and
> `renice -n -20 ${pid}` after start.
>
> Then, the IDPF driver is forced to prefer CPU0 by editing all calls
> to `queue_delayed_work`, `mod_delayed_work`, etc... to use CPU 0.
>
> Finally, `ktraces` for the workqueue events are collected.
>
> Without the current patch, the antagonist process can force
> arbitrary delays between `workqueue_queue_work` and
> `workqueue_execute_start`, that in my tests were as high as
> `30ms`. With the current patch applied, the workqueue can be
> migrated to another unloaded CPU in the same node, and, keeping
> everything else equal, the maximum delay I could see was `6us`.
>
> Fixes: 0fe45467a1041 (idpf: add create vport and netdev configuration)
> Signed-off-by: Marco Leogrande <leogrande at google.com>
> Signed-off-by: Manoj Vishwanathan <manojvishy at google.com>
Except the nit (s/praticular/particular) what Jake mentioned, changes
look good to me.
Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga at intel.com>
> ---
> drivers/net/ethernet/intel/idpf/idpf_main.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index db476b3314c8..dfd56fc5ff65 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -174,7 +174,8 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> pci_set_master(pdev);
> pci_set_drvdata(pdev, adapter);
>
> - adapter->init_wq = alloc_workqueue("%s-%s-init", 0, 0,
> + adapter->init_wq = alloc_workqueue("%s-%s-init",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0,
> dev_driver_string(dev),
> dev_name(dev));
> if (!adapter->init_wq) {
> @@ -183,7 +184,8 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_free;
> }
>
> - adapter->serv_wq = alloc_workqueue("%s-%s-service", 0, 0,
> + adapter->serv_wq = alloc_workqueue("%s-%s-service",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0,
> dev_driver_string(dev),
> dev_name(dev));
> if (!adapter->serv_wq) {
> @@ -192,7 +194,8 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_serv_wq_alloc;
> }
>
> - adapter->mbx_wq = alloc_workqueue("%s-%s-mbx", 0, 0,
> + adapter->mbx_wq = alloc_workqueue("%s-%s-mbx",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0,
> dev_driver_string(dev),
> dev_name(dev));
> if (!adapter->mbx_wq) {
> @@ -201,7 +204,8 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_mbx_wq_alloc;
> }
>
> - adapter->stats_wq = alloc_workqueue("%s-%s-stats", 0, 0,
> + adapter->stats_wq = alloc_workqueue("%s-%s-stats",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0,
> dev_driver_string(dev),
> dev_name(dev));
> if (!adapter->stats_wq) {
> @@ -210,7 +214,8 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_stats_wq_alloc;
> }
>
> - adapter->vc_event_wq = alloc_workqueue("%s-%s-vc_event", 0, 0,
> + adapter->vc_event_wq = alloc_workqueue("%s-%s-vc_event",
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0,
> dev_driver_string(dev),
> dev_name(dev));
> if (!adapter->vc_event_wq) {
More information about the Intel-wired-lan
mailing list