[PATCH] create iommu_free_table()

[Olof Johansson]

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.
> 
> Comments welcome.

Looks good, just a couple of minor nitpicks below.


-Olof


> Signed-off-by: John Rose <johnrose at austin.ibm.com>
> 
> diff -Nru a/arch/ppc64/kernel/pSeries_iommu.c b/arch/ppc64/kernel/pSeries_iommu.c
> --- a/arch/ppc64/kernel/pSeries_iommu.c	Thu Oct  7 11:08:19 2004
> +++ b/arch/ppc64/kernel/pSeries_iommu.c	Thu Oct  7 11:08:19 2004
> @@ -412,6 +412,38 @@
>  	dn->iommu_table = iommu_init_table(tbl);
>  }
>  
> +void iommu_free_table(struct device_node *dn)
> +{
> +	struct iommu_table *tbl = dn->iommu_table;
> +        unsigned long bitmap_sz, i;
> +        unsigned int order;
> +
> +        if (!tbl || !tbl->it_map) {

whitespace above looks wrong (or below?)

> +		printk(KERN_ERR "%s: expected TCE map for %s\n", __FUNCTION__,
> +				dn->full_name);
> +		return;
> +	}
> +
> +	/* verify that table contains no entries */
> +	/* it_mapsize is in entries, and we're examining 64 at a time */
> +	for (i = 0; i < (tbl->it_mapsize/64); i++) {
> +		if (tbl->it_map[i] != 0) {
> +			printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
> +				__FUNCTION__, dn->full_name);
> +			break;
> +		}

Could this get spammy? It could be nice to see a WARN_ON(1) too, so the 
call stack is dumped. If that's added, a printk_ratelimit() would 
definately be warranted around both the printk and the WARN_ON().

> +	}
> +
> +	/* calculate bitmap size in bytes */
> +	bitmap_sz = (tbl->it_mapsize + 7) / 8;
> +
> +	/* free bitmap */
> +	order = get_order(bitmap_sz);
> +	free_pages((unsigned long) tbl->it_map, order);
> +
> +	/* free table */
> +        kfree(tbl);

whitespace

> +}
>  
>  void iommu_setup_pSeries(void)
>  {
> diff -Nru a/arch/ppc64/kernel/prom.c b/arch/ppc64/kernel/prom.c
> --- a/arch/ppc64/kernel/prom.c	Thu Oct  7 11:08:19 2004
> +++ b/arch/ppc64/kernel/prom.c	Thu Oct  7 11:08:19 2004
> @@ -1818,6 +1818,9 @@
>  		return -EBUSY;
>  	}
>  
> +	if (np->iommu_table)
> +		iommu_free_table(np);
> +
>  	write_lock(&devtree_lock);
>  	OF_MARK_STALE(np);
>  	remove_node_proc_entries(np);
> diff -Nru a/include/asm-ppc64/iommu.h b/include/asm-ppc64/iommu.h
> --- a/include/asm-ppc64/iommu.h	Thu Oct  7 11:08:19 2004
> +++ b/include/asm-ppc64/iommu.h	Thu Oct  7 11:08:19 2004
> @@ -113,6 +113,9 @@
>  /* Creates table for an individual device node */
>  extern void iommu_devnode_init(struct device_node *dn);
>  
> +/* Frees table for an individual device node */
> +extern void iommu_free_table(struct device_node *dn);
> +
>  #endif /* CONFIG_PPC_MULTIPLATFORM */
>  
>  #ifdef CONFIG_PPC_ISERIES
> 
> 
> _______________________________________________
> Linuxppc64-dev mailing list
> Linuxppc64-dev at ozlabs.org
> https://ozlabs.org/cgi-bin/mailman/listinfo/linuxppc64-dev
>