Messages in this thread | | | Date | Thu, 23 Feb 2023 20:57:44 -0400 | From | Jason Gunthorpe <> | Subject | Re: [PATCH v2 02/10] iommu: Introduce a new iommu_group_replace_domain() API |
| |
On Wed, Feb 22, 2023 at 02:11:39AM +0000, Tian, Kevin wrote:
> > > There was an attempt [1] to fix error unwind in iommu_attach_group(), by > > > temporarily set group->domain to NULL before calling set_domain(). > > > > > > Jason, I wonder why this recovering cannot be done in > > > __iommu_group_set_domain() directly, e.g.: > > > > > > ret = __iommu_group_for_each_dev(group, new_domain, > > > iommu_group_do_attach_device); > > > if (ret) { > > > __iommu_group_for_each_dev(group, group->domain, > > > iommu_group_do_attach_device); > > > return ret; > > > } > > > group->domain = new_domain; > > > > We talked about this already, some times this is not the correct > > recovery case, eg if we are going to a blocking domain we need to drop > > all references to the prior domain, not put them back. > > > > Failures are WARN_ON events not error recovery. > > > > OK, I remember that. Then here looks we also need temporarily > set group->domain to NULL before calling set_domain() to recover, > as [1] does.
Sigh, this is too much.
I made a series to clean up all the domain attach logic so the error handling is all in one place and all the same.
What do you think?
https://github.com/jgunthorpe/linux/commits/iommufd_hwpt
Jason
| |