Re: [PATCH v4] x86, kdump: Set crashkernel_low automatically

[Vivek Goyal <vgoyal@redhat.com>]

On Mon, Mar 18, 2013 at 06:04:08PM -0700, Yinghai Lu wrote:
> Current code does not set low range for crashkernel if the user
> does not specify that.

Hi Yinghai,

While we are modifying changelog, it will also be beneficial to
mention that how did we end up in this situation. Can we mention
changelog little more explanatory, like as follows.

We have now modified crashkernel=X to allocate memory beyong 4G (if
available). And this will cause regression if iommu is not enabled.
Without iommu, swiotlb needs to be setup in first 4G and there is no
low memory available to second kernel.

thanks
Vivek
> 
> That cause regressions on system that does not support intel_iommu
> properly.
> 
> Chao said that his system does work well on 3.8 without extra parameter.
> even iommu does not work with kdump.
> 
> Set crashkernel_low automatically if the user does not specify that.
> 
> For system that does support IOMMU with kdump properly, user could
> specify crashkernel_low=0 to save that 72M low ram.
> 
> -v3: add swiotlb_size() according to Konrad.
> -v4: add comments what 8M is for according to hpa.
>      also update more crashkernel_low= in kernel-parameters.txt
> 
> Reported-by: WANG Chao <chaowang@redhat.com>
> Tested-by: WANG Chao <chaowang@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  Documentation/kernel-parameters.txt |   15 ++++++++++++---
>  arch/x86/kernel/setup.c             |   20 +++++++++++++++++---
>  include/linux/swiotlb.h             |    1 +
>  lib/swiotlb.c                       |   19 +++++++++++++++----
>  4 files changed, 45 insertions(+), 10 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/setup.c
> +++ linux-2.6/arch/x86/kernel/setup.c
> @@ -521,19 +521,33 @@ static void __init reserve_crashkernel_l
>  	unsigned long long low_base = 0, low_size = 0;
>  	unsigned long total_low_mem;
>  	unsigned long long base;
> +	bool auto_set = false;
>  	int ret;
>  
>  	total_low_mem = memblock_mem_size(1UL<<(32-PAGE_SHIFT));
>  	ret = parse_crashkernel_low(boot_command_line, total_low_mem,
>  						&low_size, &base);
> -	if (ret != 0 || low_size <= 0)
> -		return;
> +	if (ret != 0) {
> +		/*
> +		 * two parts from lib/swiotlb.c:
> +		 *	swiotlb size: user specified with swiotlb= or default.
> +		 *	swiotlb overflow buffer: now is hardcoded to 32k,
> +		 *		round to 8M to cover more others.
> +		 */
> +		low_size = swiotlb_size_or_default() + (8UL<<20);
> +		auto_set = true;
> +	} else {
> +		/* passed with crashkernel_low=0 ? */
> +		if (!low_size)
> +			return;
> +	}
>  
>  	low_base = memblock_find_in_range(low_size, (1ULL<<32),
>  					low_size, alignment);
>  
>  	if (!low_base) {
> -		pr_info("crashkernel low reservation failed - No suitable area found.\n");
> +		if (!auto_set)
> +			pr_info("crashkernel low reservation failed - No suitable area found.\n");
>  
>  		return;
>  	}
> Index: linux-2.6/include/linux/swiotlb.h
> ===================================================================
> --- linux-2.6.orig/include/linux/swiotlb.h
> +++ linux-2.6/include/linux/swiotlb.h
> @@ -25,6 +25,7 @@ extern int swiotlb_force;
>  extern void swiotlb_init(int verbose);
>  int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose);
>  extern unsigned long swiotlb_nr_tbl(void);
> +unsigned long swiotlb_size_or_default(void);
>  extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
>  
>  /*
> Index: linux-2.6/lib/swiotlb.c
> ===================================================================
> --- linux-2.6.orig/lib/swiotlb.c
> +++ linux-2.6/lib/swiotlb.c
> @@ -105,9 +105,9 @@ setup_io_tlb_npages(char *str)
>  	if (!strcmp(str, "force"))
>  		swiotlb_force = 1;
>  
> -	return 1;
> +	return 0;
>  }
> -__setup("swiotlb=", setup_io_tlb_npages);
> +early_param("swiotlb", setup_io_tlb_npages);
>  /* make io_tlb_overflow tunable too? */
>  
>  unsigned long swiotlb_nr_tbl(void)
> @@ -115,6 +115,18 @@ unsigned long swiotlb_nr_tbl(void)
>  	return io_tlb_nslabs;
>  }
>  EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
> +
> +/* default to 64MB */
> +#define IO_TLB_DEFAULT_SIZE (64UL<<20)
> +unsigned long swiotlb_size_or_default(void)
> +{
> +	unsigned long size;
> +
> +	size = io_tlb_nslabs << IO_TLB_SHIFT;
> +
> +	return size ? size : (IO_TLB_DEFAULT_SIZE);
> +}
> +
>  /* Note that this doesn't work with highmem page */
>  static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,
>  				      volatile void *address)
> @@ -188,8 +200,7 @@ int __init swiotlb_init_with_tbl(char *t
>  void  __init
>  swiotlb_init(int verbose)
>  {
> -	/* default to 64MB */
> -	size_t default_size = 64UL<<20;
> +	size_t default_size = IO_TLB_DEFAULT_SIZE;
>  	unsigned char *vstart;
>  	unsigned long bytes;
>  
> Index: linux-2.6/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/kernel-parameters.txt
> +++ linux-2.6/Documentation/kernel-parameters.txt
> @@ -596,9 +596,6 @@ bytes respectively. Such letter suffixes
>  			is selected automatically. Check
>  			Documentation/kdump/kdump.txt for further details.
>  
> -	crashkernel_low=size[KMG]
> -			[KNL, x86] parts under 4G.
> -
>  	crashkernel=range1:size1[,range2:size2,...][@offset]
>  			[KNL] Same as above, but depends on the memory
>  			in the running system. The syntax of range is
> @@ -606,6 +603,18 @@ bytes respectively. Such letter suffixes
>  			a memory unit (amount[KMG]). See also
>  			Documentation/kdump/kdump.txt for an example.
>  
> +	crashkernel_low=size[KMG]
> +			[KNL, x86_64] range under 4G. When crashkernel= is
> +			passed, kernel allocate physical memory region
> +			above 4G, that cause second kernel crash on system
> +			that need swiotlb later. Kernel would try to allocate
> +			some region below 4G automatically. This one let
> +			user to specify own low range under 4G for second
> +			kernel instead.
> +			0: to disable low allocation on systems that do not
> +			need swiotlb, that will save 72M low ram in first
> +			kernel.
> +
>  	cs89x0_dma=	[HW,NET]
>  			Format: <dma>
>