Thread (43 messages) flat view 43 messages, 4 authors, 2021-07-23

Re: [PATCH v2 04/14] vfio: Provide better generic support for open/release vfio_device_ops

From: Christoph Hellwig <hch@lst.de>
Date: 2021-07-23 07:39:19
Also in: intel-gfx, kvm, linux-s390

+int vfio_assign_device_set(struct vfio_device *device, void *set_id)
+{
+	struct vfio_device_set *alloc_dev_set = NULL;
+	struct vfio_device_set *dev_set;
+
+	if (WARN_ON(!set_id))
+		return -EINVAL;
+
+	/*
+	 * Atomically acquire a singleton object in the xarray for this set_id
+	 */
+again:
+	xa_lock(&vfio_device_set_xa);
+	if (alloc_dev_set) {
+		dev_set = __xa_cmpxchg(&vfio_device_set_xa,
+				       (unsigned long)set_id, NULL,
+				       alloc_dev_set, GFP_KERNEL);
+		if (xa_is_err(dev_set)) {
+			xa_unlock(&vfio_device_set_xa);
+			kfree(alloc_dev_set);
+			return xa_err(dev_set);
+		}
+		if (!dev_set)
+			dev_set = alloc_dev_set;
+	} else {
+		dev_set = xa_load(&vfio_device_set_xa, (unsigned long)set_id);
+	}
+
+	if (dev_set) {
+		dev_set->device_count++;
+		xa_unlock(&vfio_device_set_xa);
+		device->dev_set = dev_set;
+		if (dev_set != alloc_dev_set)
+			kfree(alloc_dev_set);
+		return 0;
+	}
+	xa_unlock(&vfio_device_set_xa);
+
+	if (WARN_ON(alloc_dev_set))
+		return -EINVAL;
+
+	alloc_dev_set = kzalloc(sizeof(*alloc_dev_set), GFP_KERNEL);
+	if (!alloc_dev_set)
+		return -ENOMEM;
+	mutex_init(&alloc_dev_set->lock);
+	alloc_dev_set->set_id = set_id;
+	goto again;
+}
+EXPORT_SYMBOL_GPL(vfio_assign_device_set);
This looks unessecarily complicated.  We can just try to load first
and then store it under the same lock, e.g.:

int vfio_assign_device_set(struct vfio_device *device, void *set_id)
{
	unsigned long idx = (unsigned long)set_id;
	struct vfio_device_set *set, *new;
	int err;

	if (WARN_ON(!set_id))
		return -EINVAL;

	xa_lock(&vfio_device_set_xa);
	set = xa_load(&vfio_device_set_xa, idx);
	if (set)
		goto found;
	xa_unlock(&vfio_device_set_xa);

	new = kzalloc(sizeof(*new), GFP_KERNEL);
	if (!new)
		return -ENOMEM;
	mutex_init(&new->lock);
	alloc_dev_set->set_id = set_id;

	xa_lock(&vfio_device_set_xa);
	set = xa_load(&vfio_device_set_xa, idx);
	if (set) {
		kfree(new);
		goto found;
	}
	err = xa_err(__xa_store(&vfio_device_set_xa, idx, new, GFP_KERNEL));
	xa_unlock(&vfio_device_set_xa);
	if (err)
		kfree(new);
	return err;

found:
	set->device_count++;
	xa_unlock(&vfio_device_set_xa);

	device->dev_set = set;
	return 0;
}
+static void vfio_release_device_set(struct vfio_device *device)
+{
+	struct vfio_device_set *dev_set = device->dev_set;
+
+	if (!dev_set)
+		return;
+
+	xa_lock(&vfio_device_set_xa);
+	dev_set->device_count--;
+	if (!dev_set->device_count) {
Nit, by I'd find

	if (!--dev_set->device_count) {

easier to follow as it clearly documents the dec_and_test pattern.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help