Re: [PATCH v5 2/7] s390: ap: new vfio_ap_queue structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/03/2019 11:33, Cornelia Huck wrote:
On Wed, 13 Mar 2019 17:04:59 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
index e9824c3..df6f21a 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -40,14 +40,42 @@ static struct ap_device_id ap_queue_ids[] = {
MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids); +/**
+ * vfio_ap_queue_dev_probe:
+ *
+ * Allocate a vfio_ap_queue structure and associate it
+ * with the device as driver_data.
+ */
  static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
  {
+	struct vfio_ap_queue *q;
+
+	q = kzalloc(sizeof(*q), GFP_KERNEL);
+	if (!q)
+		return -ENOMEM;
+	dev_set_drvdata(&apdev->device, q);
+	q->apqn = to_ap_queue(&apdev->device)->qid;
+	INIT_LIST_HEAD(&q->list);
+	mutex_lock(&matrix_dev->lock);
+	list_add(&q->list, &matrix_dev->free_list);
+	mutex_unlock(&matrix_dev->lock);

 From what I can see, dealing with the free_list is supposed to be
protected by the matrix_dev mutex, and at a glance, it indeed seems to
be held every time you interact with the list. I think it would be good
to document that with a comment.

Yes.
It is a big lock but only on administrative tasks so I think it is harmless.


(I have not reviewed this deeply, and I won't be able to look at this
more until April, sorry.)

OK.
Thanks for the comments.

Regards,
Pierre


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux