Re: 2.5.73-mm2

[Christoph Hellwig]

On Sat, Jun 28, 2003 at 08:54:36AM -0700, William Lee Irwin III wrote:
> +config HIGHPMD
> +	bool "Allocate 2nd-level pagetables from highmem"
> +	depends on HIGHMEM64G
> +	help
> +	  The VM uses one pmd entry for each pagetable page of physical
> +	  memory allocated. For systems with extreme amounts of highmem,
> +	  this cannot be tolerated. Setting this option will put
> +	  userspace 2nd-level pagetables in highmem.

Does this make sense for !HIGHPTE?  In fact does it make sense to
carry along HIGHPTE as an option still? ..

> +#ifndef CONFIG_HIGHPMD /* Oh boy. Error reporting is going to blow major goats. */

Any chance you can rearragne the code to avoid the ifndef in favour
of an ifdef?

>  		set_pte(dst_pte, entry);
> +		pmd_unmap(dst_pte);
> +		pmd_unmap_nested(src_pte);

<Lots more pmd_unmap* calls snipped>

Looks like you changed some API so that pmds are now returned mapped?
It might make sense to change their names into foo_map then so the
breakage is at the API level if someone misses updates for the changes.

> +#ifdef CONFIG_HIGHPMD
> +#define	GFP_PMD		(__GFP_REPEAT|__GFP_HIGHMEM|GFP_KERNEL)
> +#else
> +#define GFP_PMD		(__GFP_REPEAT|GFP_KERNEL)
> +#endif

So what?  Do you want to use a space or tab after the #define? :)

Also Given that GFP_PMD is used just once it's argueable whether it makes
sense to get rid of the defintion and use the expanded values directly.

Otherwise the patch looks fine to me and should allow to get some more
free lowmem on those insanely big 32bit machines.. :)