Re: [PATCH v16 02/14] s390/vfio-ap: use new AP bus interface to search for queue devices

Jason J. Herne
2021-06-09T13:52:37Z

On 5/10/21 12:44 PM, Tony Krowiak wrote:
> This patch refactors the vfio_ap device driver to use the AP bus's
> ap_get_qdev() function to retrieve the vfio_ap_queue struct containing
> information about a queue that is bound to the vfio_ap device driver.
> The bus's ap_get_qdev() function retrieves the queue device from a
> hashtable keyed by APQN. This is much more efficient than looping over
> the list of devices attached to the AP bus by several orders of
> magnitude.
>
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 757166da947e..8a50aa650b65 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -27,13 +27,6 @@
> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev);
> static struct vfio_ap_queue *vfio_ap_find_queue(int apqn);
>
> -static int match_apqn(struct device *dev, const void *data)
> -{
> - struct vfio_ap_queue *q = dev_get_drvdata(dev);
> -
> - return (q->apqn == *(int *)(data)) ? 1 : 0;
> -}
> -
> /**
> * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list
> * @matrix_mdev: the associated mediated matrix
> @@ -1253,15 +1246,17 @@ static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
>
> static struct vfio_ap_queue *vfio_ap_find_queue(int apqn)
> {
> - struct device *dev;
> + struct ap_queue *queue;
> struct vfio_ap_queue *q = NULL;

The use of q and queue as variable names was a little confusing to me at first. I tried
renaming them a few times, the best I could come up with was this:

struct ap_queue *queue;
struct vfio_ap_queue *vfio_queue = NULL;

Take it or leave it :) Other than that, LGTM.
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>

> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL,
> - &apqn, match_apqn);
> - if (dev) {
> - q = dev_get_drvdata(dev);
> - put_device(dev);
> - }
> + queue = ap_get_qdev(apqn);
> + if (!queue)
> + return NULL;
> +
> + if (queue->ap_dev.device.driver == &matrix_dev->vfio_ap_drv->driver)
> + q = dev_get_drvdata(&queue->ap_dev.device);
> +
> + put_device(&queue->ap_dev.device);
>
> return q;
> }
>

--
-- Jason J. Herne (jjherne@linux.ibm.com)