On 09.08.2023 14:15, Jason Gunthorpe wrote:
> On Wed, Aug 09, 2023 at 04:01:42AM +0000, Tian, Kevin wrote:
>>> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
>>> Sent: Wednesday, August 9, 2023 1:27 AM
>>>
>>> @@ -1800,11 +1801,18 @@ struct probe_iommu_args {
>>> static int probe_iommu_group(struct device *dev, void *data)
>>> {
>>> struct probe_iommu_args *args = data;
>>> + bool need_lock;
>>> int ret;
>>>
>>> - device_lock(dev);
>>> + /* Probing the iommu itself is always done under the device_lock */
>>> + need_lock = !args->iommu || args->iommu->hwdev != dev;
>>> +
>> is !args->iommu a valid case?
> Hmm, not any more, it used to happen in an earlier version
>
>> btw probably a dumb question. Why do we continue to probe the
>> iommu device itself instead of skipping it? The group is a concept
>> for devices which require DMA protection from iommu instead of
>> for the iommu device itself...
> Yeah, that is how I originally did it, but since the locking appeared
> here I thought it would be safer to just continue to invoke probe as
> we have always done. I don't know for sure that there isn't some
> driver that relies on this for some reason.
>
> eg it might change the group layouts or something.
Well, I don't think that there is any driver doing such things, but the
only way to check is simply change the behavior an wait for complaints.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
