[PATCH] (RFC) slaughter imalloc

[Benjamin Herrenschmidt]

On Tue, 2005-05-17 at 15:15 -0500, John Rose wrote:
> Hi Ben-
> 
> Great job!  The patch cleans things up nicely.
> 
> For PHB removal, iounmap_explicit() will be called for the range of the
> PHB.  In many cases, this range will have gaps of invalidated PTEs from
> children slots that have already been removed.  Will unmap_vm_area()
> and/or the invalidate_pte hcall tolerate such gaps?  I don't know
> enough to answer that w/o actually testing it.

It should be ok, empty pgd/pud/pmd's are skipped, and
ptep_test_and_clear() should be harmless. The hash flushing routines
don't mind neither. The only thing in there I see that test the state of
the actual PTE is that bit:

	WARN_ON(!pte_none(ptent) && !pte_present(ptent));

Which I think won't trigger, it's supposed to catch a non-present PTE
that isn't also empty (that is a swap PTE I suppose) which will never
happen in our case.

> While we're cleaning stuff up, we might look at naming the explicit
> functions consistently.  I wrote them, so I'm the guilty party :)
> __ioremap_explicit <-> iounmap_explicit, etc.

Ok, though at the same time, those are really "internal" APIs not to be
used by the common mortals so the __ does make some sense. I'll think
about it.

> Here are some more comments:
> 
> +/*
> + * map_io_page currently only called by __ioremap
> 
> Not totally accurate, since explicit() calls it too :)  I should have 
> removed this line...

Hehe, will look, it may even be called elsewhere.

> + * map_io_page adds an entry to the ioremap page table
> + * and adds an entry to the HPT, possibly bolting it
> + */
> +static int map_io_page(unsigned long ea, unsigned long pa, int flags)
> +{
> +	pgd_t *pgdp;
> +	pud_t *pudp;
> +	pmd_t *pmdp;
> +	pte_t *ptep;
> +	unsigned long vsid;
> +
> +	if (mem_init_done) {
> +		spin_lock(&init_mm.page_table_lock);
> +		pgdp = pgd_offset_k(ea);
> +		pudp = pud_alloc(&init_mm, pgdp, ea);
> +		if (!pudp)
> +			return -ENOMEM;
> +		pmdp = pmd_alloc(&init_mm, pudp, ea);
> +		if (!pmdp)
> +			return -ENOMEM;
> +		ptep = pte_alloc_kernel(&init_mm, pmdp, ea);
> +		if (!ptep)
> +			return -ENOMEM;
> +		pa = abs_to_phys(pa);
> +		set_pte_at(&init_mm, ea, ptep, pfn_pte(pa >> PAGE_SHIFT,
> +							  __pgprot(flags)));
> +		spin_unlock(&init_mm.page_table_lock);
> 
> Now that we're so vmalloc-hip, could we use map_vm_area() here?

I don't think map_vm_area() is suitable for ioremap like thigns. It
wants struct page pointer arrays and such things. Even x86 doesn't use
it.

> +EXPORT_SYMBOL(ioremap_bot); /* poor XFS ... */
> 
> Does XFS really use this!?

XFS has some hacks using VMALLOC_START which is now defined as using
ioremap_bot.

Ben.