On Thu, May 14, 2020 at 11:12:52PM +0000, Prakhya, Sai Praneeth wrote:
> +static int is_driver_bound(struct device *dev, void *not_used)
> +{
> + int ret = 0;
> +
> + device_lock(dev);
> + if (device_is_bound(dev))
> + ret = 1;
> + device_unlock(dev);
> + return ret;
> +}
This locks only one device, so without lock-conversion there could be a
driver probe after the device_unlock(), while we are probing the other
devices of the group.
> [SNIP]
>
> + /*
> + * Check if any device in the group still has a driver binded to it.
> + * This might race with device driver probing code and unfortunately
> + * there is no clean way out of that either, locking all devices in the
> + * group and then do the re-attach will introduce a lock-inversion with
> + * group->mutex - Joerg.
> + */
> + if (iommu_group_for_each_dev(group, NULL, is_driver_bound)) {
> + pr_err("Active drivers exist for devices in the group\n");
> + return -EBUSY;
> + }
The lock inversion comes into the picture when this code is called from
device(-driver) core through the bus-notifiers. The notifiers are called
with the device already locked.
> Another question I have is.. if it's racy then it should be racy even
> for one device iommu groups.. right? Why would it be racy only with
> multiple devices iommu group?
Valid point. So the device needs to be locked _while_ the default domain
change happens. If triggered by sysfs there should be no locking
problems, I guess. But you better try it out.
Regards,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
