Re: [PATCH] x86: create a non-zero sized bm_pte only when needed

[Jeremy Fitzhardinge <jeremy@goop.org>]

Jan Beulich wrote:
> Impact: kernel image size reduction
>
> Since in most configurations the pmd page needed maps the same range of
> virtual addresses which is also mapped by the earlier inserted one for
> covering FIX_DBGP_BASE, that page (and its insertion in the page
> tables) can be avoided altogether by detecting the condition at compile
> time.
>   

Does this depend on CONFIG_EARLY_PRINTK_DBGP being set?  And what's so 
special about FIX_DBGP_BASE, that we should hard-code it in here?  Is it 
just that its the first non-arch-dependent fixmap slot?  Or something 
else?  Will it break if we move FIX_DBGP_BASE?

Is the space saving here just the 1 page for bm_pte[]?  Wouldn't we do 
as well by making it initdata?

I'm picking on this change because its breaking Xen PV booting...

>  static __initdata int after_paging_init;
> -static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)] __page_aligned_bss;
> +#define __FIXADDR_TOP (-PAGE_SIZE)
>   

Will this break in a 32-bit PV kernel where FIXADDR_TOP is shifted?

This seriously needs a good inline comment.

> +static pte_t bm_pte[(__fix_to_virt(FIX_DBGP_BASE)
> +		     ^ __fix_to_virt(FIX_BTMAP_BEGIN)) >> PMD_SHIFT
> +		    ? PAGE_SIZE / sizeof(pte_t) : 0] __page_aligned_bss;
> +#undef __FIXADDR_TOP
> +static __initdata pte_t *bm_ptep;
>  
>  static inline pmd_t * __init early_ioremap_pmd(unsigned long addr)
>  {
> @@ -505,6 +510,8 @@ static inline pmd_t * __init early_iorem
>  
>  static inline pte_t * __init early_ioremap_pte(unsigned long addr)
>  {
> +	if (!sizeof(bm_pte))
> +		return &bm_ptep[pte_index(addr)];
>  	return &bm_pte[pte_index(addr)];
>   

Why not just assign bm_ptep = bm_pte if we're using the array?

>  }
>  
> @@ -516,8 +523,14 @@ void __init early_ioremap_init(void)
>  		printk(KERN_INFO "early_ioremap_init()\n");
>  
>  	pmd = early_ioremap_pmd(fix_to_virt(FIX_BTMAP_BEGIN));
> -	memset(bm_pte, 0, sizeof(bm_pte));
> -	pmd_populate_kernel(&init_mm, pmd, bm_pte);
> +	if (sizeof(bm_pte)) {
> +		memset(bm_pte, 0, sizeof(bm_pte));
> +		pmd_populate_kernel(&init_mm, pmd, bm_pte);
> +	} else {
> +		bm_ptep = pte_offset_kernel(pmd, 0);
> +		if (early_ioremap_debug)
> +			printk(KERN_INFO "bm_ptep=%p\n", bm_ptep);
> +	}

    J