On Wed, Jan 11, 2023 at 07:19:34PM +0100, Thomas Gleixner wrote: > On Thu, Jan 05 2023 at 15:33, Jason Gunthorpe wrote: > > + > > + mutex_lock(&group->mutex); > > + list_for_each_entry(group_dev, &group->devices, list) > > + ret &= msi_device_has_isolated_msi(group_dev->dev) || > > + device_iommu_capable(group_dev->dev, > > + IOMMU_CAP_INTR_REMAP); > > Nit. This really wants brackets even if they are not required by the > language. Why? > > Brackets can be omitted for a single line statement in the loop/if path, > but this > > > + ret &= msi_device_has_isolated_msi(group_dev->dev) || > > + device_iommu_capable(group_dev->dev, > > + IOMMU_CAP_INTR_REMAP); > > is visually a multi line statement. So having brackets makes visual > parsing of this construct way clearer: Sure, though this is all undone by the last patch which does make it visually a single statement: list_for_each_entry(group_dev, &group->devices, list) - ret &= msi_device_has_isolated_msi(group_dev->dev) || - device_iommu_capable(group_dev->dev, - IOMMU_CAP_INTR_REMAP); + ret &= msi_device_has_isolated_msi(group_dev->dev); > Also get rid of that extra line break. We lifted the 80 characters line > length quite some while ago and made it 100. Ah, there are so many different opinions on that! I got everything else, I'll stick this in linux-next via the iommufd tree so more bots can check it - if anyone else has remarks I can make a v4 Thanks, Jason
