Messages in this thread | | | Date | Tue, 20 Jul 2021 19:41:02 +1000 | Subject | Re: [PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper | From | Alexey Kardashevskiy <> |
| |
On 20/07/2021 15:38, Leonardo Brás wrote: > Hello Fred, thanks for this feedback! > > Sorry if I miss anything, this snippet was written for v1 over an year > ago, and I have not taken a look at it ever since. > > On Mon, 2021-07-19 at 15:53 +0200, Frederic Barrat wrote: >> >> >> On 16/07/2021 10:27, Leonardo Bras wrote: >>> @@ -1099,18 +1105,13 @@ int iommu_take_ownership(struct iommu_table >>> *tbl) >>> for (i = 0; i < tbl->nr_pools; i++) >>> spin_lock_nest_lock(&tbl->pools[i].lock, &tbl- >>>> large_pool.lock); >>> >>> - iommu_table_release_pages(tbl); >>> - >>> - if (!bitmap_empty(tbl->it_map, tbl->it_size)) { >>> + if (iommu_table_in_use(tbl)) { >>> pr_err("iommu_tce: it_map is not empty"); >>> ret = -EBUSY; >>> - /* Undo iommu_table_release_pages, i.e. restore >>> bit#0, etc */ >>> - iommu_table_reserve_pages(tbl, tbl- >>>> it_reserved_start, >>> - tbl->it_reserved_end); >>> - } else { >>> - memset(tbl->it_map, 0xff, sz); >>> } >>> >>> + memset(tbl->it_map, 0xff, sz); >>> + >> >> >> So if the table is not empty, we fail (EBUSY) but we now also >> completely >> overwrite the bitmap. It was in an unexpected state, but we're making >> it >> worse. Or am I missing something? > > IIRC there was a reason to do that at the time, but TBH I don't really > remember it, and by looking at the code right now you seem to be > correct about this causing trouble. > > I will send a v6 fixing it soon. > Please review the remaining patches for some issue I may be missing. > > Alexey, any comments on that?
Agree with Fred, this is a bug, EBUSY is not that unexpected :-/ Thanks,
> >> >> Fred >> > > Again, thank you for reviewing Fred! > Best regards, > Leonardo Bras > > > > >
-- Alexey
| |