problems with iommu_free_table()

[Benjamin Herrenschmidt]

On Thu, 2004-10-07 at 12:54 -0500, John Rose wrote:
> The patch below creates iommu_free_table().  Iommu tables are not currently
> freed in PPC64.  This could cause a memory leak for DLPAR of an EADS slot.  The
> function verifies that there are no outstanding TCE entries for the range of
> the table before freeing it.  I added a call to iommu_free_table() to the code
> that dynamically removes a device node.  This should be fairly symmetrical with
> the table allocation, which happens during dynamic addition of a device node.

Ouch, I should have commented earlier... now it went in and has
problems:

 - It breaks build without CONFIG_PPC_PSERIES (try a pmac-only build).
There is, more generally, a tendency at calling things in
pSeries_iommu.c with the prefix "iommu_" without any mention of
"pSeries" in the name. Hey guys ! pSeries isn't alone anymore ! So
please call those pSeries-specific things pSeries_* or tce_* or
whatever, but don't add back confusion where I had such a hard time
splitting things.

 - It seems that any call to of_remove_node() will call
iommu_free_table() on np->iommu_table. That sounds bad. The iommu_table
pointer is copied at init time from the parent to all child nodes. So if
we add a phb, and then remove a device from that bus, we end up
disposing of the phb's iommu table ...

I'll send a patch fixing G5 build by renaming iommu_free_table to
tce_free_table() and putting the call in #ifdef CONFIG_PPC_PSERIES for
now, but if you start hooking too much between prom.c and the higher
level, you should start thinking about doing things differently. That is
have of_remove_node() stay what it should have been from the beginning:
a low level function removing the node and just that, and have the
_caller_ to the grunt work of knowing what else need to be
removed/freed/etc...

Ben.