Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically

On Wed, Jun 25, 2014 at 05:56:37PM +1000, Benjamin Herrenschmidt wrote:

On Wed, 2014-06-25 at 17:50 +1000, Alexey Kardashevskiy wrote:

quoted

quoted

Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
need to release the iommu table. Currently, there is one function to release
the iommu table, iommu_free_table() which takes a pointer of the iommu_table
and release it.

If the iommu table in PE is just a part of PE, it will have some problem to
release it with iommu_free_table(). That's why I make it a pointer in PE
structure.

So you are saying that you want to release PE by one kfree() and release
iommu_table by another kfree (embedded into iommu_free_table()). For me
that means that PE and iommu_table have different lifetime.

And I cannot find the exact place in this patchset where you call
iommu_free_table(), what do I miss?

He has a point though... iommu_free_table() does a whole bunch of things
in addition to kfree at the end.

This is a discrepancy in the iommu.c code, we don't allocate the table,
it's allocated by our callers, but we do free it in iommu_free_table().

My gut feeling is that we should fix that in the core by moving the
kfree() out of iommu_free_table() and back into vio.c and
pseries/iommu.c, the only two callers, otherwise we can't wrap the table
structure inside another object if we are going to ever free it.

Yes, this is another option. Move the kfree() outside could keep some logic in
current code, like in pnv_pci_ioda_tce_invalidate(). We could get the tbl from
a PE structure directly, instead of adding a field in tbl to point to the PE
structure.

Cheers,
Ben.

-- 
Richard Yang
Help you, Help me