Re: [PATCH v2 02/16] s390/vfio-ap: Provide access to queue objects and related info

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inefficient O(N*M) loop to calculate the number of queues instead of a constant-time weight calculation.
--

--- Patch [2]: [PATCH v2 02/16] s390/vfio-ap: Provide access to queue objects and related info ---
commit ead4ee761bf33471a8b6bc2e8080a705110bd528
Author: Anthony Krowiak <[email protected]>

s390/vfio-ap: Provide access to queue objects and related info

This commit provides functions to access vfio_ap_queue objects and to return
the number of queues passed through to a guest. It makes an existing queue
retrieval function non-static, adds a lockdep assertion, and introduces a
new function to count the total queues.

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index ef3101f6865e5..cd85866b81a05 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c

[ ... ]

> @@ -2098,6 +2098,20 @@ static void vfio_ap_mdev_request(struct vfio_device *vdev, unsigned int count)
>  	release_update_locks_for_mdev(matrix_mdev);
>  }
>  
> +int vfio_ap_mdev_get_num_queues(struct ap_matrix *ap_matrix)
> +{
> +	unsigned long apid, apqi;
> +	int num_queues = 0;
> +
> +	lockdep_assert_held(&matrix_dev->mdevs_lock);
> +
> +	for_each_set_bit_inv(apid, ap_matrix->apm, AP_DEVICES)
> +		for_each_set_bit_inv(apqi, ap_matrix->aqm, AP_DOMAINS)
> +			num_queues++;

[Severity: Medium]
Since the total number of AP queues is determined by the Cartesian product of
the set bits in these two matrices, could this be calculated more efficiently
without a nested loop?

For example, utilizing bitmap_weight() would avoid the iterations entirely:
bitmap_weight(ap_matrix->apm, AP_DEVICES) * bitmap_weight(ap_matrix->aqm, AP_DOMAINS)

The current nested loop executes an iteration for every set bit combination 
while holding the mdevs_lock, whereas the bitmap_weight approach would achieve
the same result in a few word-level popcount operations.

> +
> +	return num_queues;
> +}
> +
>  static int vfio_ap_mdev_get_device_info(unsigned long arg)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2