Re: [PATCH v2 1/4] x86, kdump: Set crashkernel_low automatically

[HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>]

(2013/04/04 9:38), Yinghai Lu wrote:

> 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;

The variable default_size seems no longer necessary. IO_TLB_DEFAULT_SIZE
should be used directly.

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

In fact, swiotlb caused the 2nd kernel to fail to boot this time, but in
general, other components that require low memory can cause the same
situation. I think it better to avoid the description that only swiotlb
can cause the 2nd kernel to fail to boot.

So, how about this?

... that cause second kernel crash on system that require some amount of
low memory, e.g. swiotlb that requires at least 72MB low memory at
default. Kernel would ...

> +			0: to disable low allocation on systems that do not
> +			need swiotlb, that will save 72M low ram in first
> +			kernel.
> +

Similarly, I think it better not to say swiotlb thing here. Just:

  0: to disable automatic low memory allocation.

Thanks.
HATAYAMA, Daisuke