[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