[PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[Xunlei Pang]

"/sys/kernel/kexec_crash_size" only handles crashk_res, it
is fine in most cases, but sometimes we have crashk_low_res.
For example, when "crashkernel=size[KMG],high" combined with
"crashkernel=size[KMG],low" is used for 64-bit x86.

Like crashk_res, we introduce the corresponding sysfs file
"/sys/kernel/kexec_crash_low_size" for crashk_low_res.

So, the exact total reserved memory is the sum of the two.

crashk_low_res can also be shrunk via this new interface,
and users should be aware of what they are doing.

Suggested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
 include/linux/kexec.h |  4 ++--
 kernel/kexec_core.c   | 23 ++++++++++++-----------
 kernel/ksysfs.c       | 25 +++++++++++++++++++++++--
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d743777..4f271fc 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -304,8 +304,8 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base);
 int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
 		unsigned long long *crash_size, unsigned long long *crash_base);
-int crash_shrink_memory(unsigned long new_size);
-size_t crash_get_memory_size(void);
+int crash_shrink_memory(struct resource *res, unsigned long new_size);
+size_t crash_get_memory_size(struct resource *res);
 void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
 
 int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..707d18e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -925,13 +925,13 @@ void crash_kexec(struct pt_regs *regs)
 	}
 }
 
-size_t crash_get_memory_size(void)
+size_t crash_get_memory_size(struct resource *res)
 {
 	size_t size = 0;
 
 	mutex_lock(&kexec_mutex);
-	if (crashk_res.end != crashk_res.start)
-		size = resource_size(&crashk_res);
+	if (res->end != res->start)
+		size = resource_size(res);
 	mutex_unlock(&kexec_mutex);
 	return size;
 }
@@ -945,7 +945,7 @@ void __weak crash_free_reserved_phys_range(unsigned long begin,
 		free_reserved_page(boot_pfn_to_page(addr >> PAGE_SHIFT));
 }
 
-int crash_shrink_memory(unsigned long new_size)
+int crash_shrink_memory(struct resource *res, unsigned long new_size)
 {
 	int ret = 0;
 	unsigned long start, end;
@@ -958,8 +958,9 @@ int crash_shrink_memory(unsigned long new_size)
 		ret = -ENOENT;
 		goto unlock;
 	}
-	start = crashk_res.start;
-	end = crashk_res.end;
+
+	start = res->start;
+	end = res->end;
 	old_size = (end == 0) ? 0 : end - start + 1;
 	if (new_size >= old_size) {
 		ret = (new_size == old_size) ? 0 : -EINVAL;
@@ -975,17 +976,17 @@ int crash_shrink_memory(unsigned long new_size)
 	start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
 	end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
 
-	crash_free_reserved_phys_range(end, crashk_res.end);
+	crash_free_reserved_phys_range(end, res->end);
 
-	if ((start == end) && (crashk_res.parent != NULL))
-		release_resource(&crashk_res);
+	if ((start == end) && (res->parent != NULL))
+		release_resource(res);
 
 	ram_res->start = end;
-	ram_res->end = crashk_res.end;
+	ram_res->end = res->end;
 	ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
 	ram_res->name = "System RAM";
 
-	crashk_res.end = end - 1;
+	res->end = end - 1;
 
 	insert_resource(&iomem_resource, ram_res);
 
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index ee1bc1b..3336fd5 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -105,10 +105,30 @@ static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 
+static ssize_t kexec_crash_low_size_show(struct kobject *kobj,
+				       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%zu\n", crash_get_memory_size(&crashk_low_res));
+}
+static ssize_t kexec_crash_low_size_store(struct kobject *kobj,
+				   struct kobj_attribute *attr,
+				   const char *buf, size_t count)
+{
+	unsigned long cnt;
+	int ret;
+
+	if (kstrtoul(buf, 0, &cnt))
+		return -EINVAL;
+
+	ret = crash_shrink_memory(&crashk_low_res, cnt);
+	return ret < 0 ? ret : count;
+}
+KERNEL_ATTR_RW(kexec_crash_low_size);
+
 static ssize_t kexec_crash_size_show(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%zu\n", crash_get_memory_size());
+	return sprintf(buf, "%zu\n", crash_get_memory_size(&crashk_res));
 }
 static ssize_t kexec_crash_size_store(struct kobject *kobj,
 				   struct kobj_attribute *attr,
@@ -120,7 +140,7 @@ static ssize_t kexec_crash_size_store(struct kobject *kobj,
 	if (kstrtoul(buf, 0, &cnt))
 		return -EINVAL;
 
-	ret = crash_shrink_memory(cnt);
+	ret = crash_shrink_memory(&crashk_res, cnt);
 	return ret < 0 ? ret : count;
 }
 KERNEL_ATTR_RW(kexec_crash_size);
@@ -218,6 +238,7 @@ static struct attribute * kernel_attrs[] = {
 #ifdef CONFIG_KEXEC_CORE
 	&kexec_loaded_attr.attr,
 	&kexec_crash_loaded_attr.attr,
+	&kexec_crash_low_size_attr.attr,
 	&kexec_crash_size_attr.attr,
 	&vmcoreinfo_attr.attr,
 #endif
-- 
1.8.3.1

[PATCH v2 2/2] kexec: Consider crashk_low_res in sanity_check_segment_list()

[Xunlei Pang]

We have crashk_res only in most cases, but sometimes we have
crashk_low_res.

For example, on 64-bit x86 systems, when "crashkernel=32M,high"
combined with "crashkernel=128M,low" is used, so some segments
may have the chance to be loaded into crashk_low_res area. We
can't fail it as a memory violation in these cases.

Thus, we add the case to regard the segment as valid if it is
within crashk_low_res.

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
 kernel/kexec_core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 707d18e..9012a60 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -248,9 +248,14 @@ int sanity_check_segment_list(struct kimage *image)
 			mstart = image->segment[i].mem;
 			mend = mstart + image->segment[i].memsz - 1;
 			/* Ensure we are within the crash kernel limits */
-			if ((mstart < phys_to_boot_phys(crashk_res.start)) ||
-			    (mend > phys_to_boot_phys(crashk_res.end)))
-				return -EADDRNOTAVAIL;
+			if ((mstart >= phys_to_boot_phys(crashk_res.start)) &&
+			    (mend <= phys_to_boot_phys(crashk_res.end)))
+				continue;
+			if ((mstart >= phys_to_boot_phys(crashk_low_res.start)) &&
+			    (mend <= phys_to_boot_phys(crashk_low_res.end)))
+				continue;
+
+			return -EADDRNOTAVAIL;
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH v2 2/2] kexec: Consider crashk_low_res in sanity_check_segment_list()

[Dave Young]

Hi, Xunlei,

On 08/17/16 at 09:50am, Xunlei Pang wrote:
> We have crashk_res only in most cases, but sometimes we have
> crashk_low_res.
> 
> For example, on 64-bit x86 systems, when "crashkernel=32M,high"
> combined with "crashkernel=128M,low" is used, so some segments
> may have the chance to be loaded into crashk_low_res area. We
> can't fail it as a memory violation in these cases.
> 
> Thus, we add the case to regard the segment as valid if it is
> within crashk_low_res.

crashkernel low is meant for swiotlb, it can be reserved automaticlly
in case there's only crashkernel high specified in cmdline, I'm not
sure it is useful to use crashk_res_low for other purpose and
likely kdump can fail in the case. 

I'm not sure it is really necessary to add this check now, we may
handle it only when there is an actual use case and bug report in
the future.

Thanks
Dave
> 
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>  kernel/kexec_core.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 707d18e..9012a60 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -248,9 +248,14 @@ int sanity_check_segment_list(struct kimage *image)
>  			mstart = image->segment[i].mem;
>  			mend = mstart + image->segment[i].memsz - 1;
>  			/* Ensure we are within the crash kernel limits */
> -			if ((mstart < phys_to_boot_phys(crashk_res.start)) ||
> -			    (mend > phys_to_boot_phys(crashk_res.end)))
> -				return -EADDRNOTAVAIL;
> +			if ((mstart >= phys_to_boot_phys(crashk_res.start)) &&
> +			    (mend <= phys_to_boot_phys(crashk_res.end)))
> +				continue;
> +			if ((mstart >= phys_to_boot_phys(crashk_low_res.start)) &&
> +			    (mend <= phys_to_boot_phys(crashk_low_res.end)))
> +				continue;
> +
> +			return -EADDRNOTAVAIL;
>  		}
>  	}
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 2/2] kexec: Consider crashk_low_res in sanity_check_segment_list()

[Xunlei Pang]

On 2016/08/17 at 15:24, Dave Young wrote:
> Hi, Xunlei,
>
> On 08/17/16 at 09:50am, Xunlei Pang wrote:
>> We have crashk_res only in most cases, but sometimes we have
>> crashk_low_res.
>>
>> For example, on 64-bit x86 systems, when "crashkernel=32M,high"
>> combined with "crashkernel=128M,low" is used, so some segments
>> may have the chance to be loaded into crashk_low_res area. We
>> can't fail it as a memory violation in these cases.
>>
>> Thus, we add the case to regard the segment as valid if it is
>> within crashk_low_res.
> crashkernel low is meant for swiotlb, it can be reserved automaticlly
> in case there's only crashkernel high specified in cmdline, I'm not
> sure it is useful to use crashk_res_low for other purpose and
> likely kdump can fail in the case. 
>
> I'm not sure it is really necessary to add this check now, we may
> handle it only when there is an actual use case and bug report in
> the future.

Thanks for the review.
The reason I added this is that crashk_res is allowed to be shrunk, so the segment
will surely fall into crashk_low_res if crashk_res was shrunk to be a small range.

But yes, this should be a corner case, but seems it does no harm adding this check.
Anyway, if you think it's not necessary, let's simply ignore it :-)

Regards,
Xunlei

>
> Thanks
> Dave
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>> ---
>>  kernel/kexec_core.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index 707d18e..9012a60 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -248,9 +248,14 @@ int sanity_check_segment_list(struct kimage *image)
>>  			mstart = image->segment[i].mem;
>>  			mend = mstart + image->segment[i].memsz - 1;
>>  			/* Ensure we are within the crash kernel limits */
>> -			if ((mstart < phys_to_boot_phys(crashk_res.start)) ||
>> -			    (mend > phys_to_boot_phys(crashk_res.end)))
>> -				return -EADDRNOTAVAIL;
>> +			if ((mstart >= phys_to_boot_phys(crashk_res.start)) &&
>> +			    (mend <= phys_to_boot_phys(crashk_res.end)))
>> +				continue;
>> +			if ((mstart >= phys_to_boot_phys(crashk_low_res.start)) &&
>> +			    (mend <= phys_to_boot_phys(crashk_low_res.end)))
>> +				continue;
>> +
>> +			return -EADDRNOTAVAIL;
>>  		}
>>  	}
>>  
>> -- 
>> 1.8.3.1
>>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[Dave Young]

On 08/17/16 at 09:50am, Xunlei Pang wrote:
> "/sys/kernel/kexec_crash_size" only handles crashk_res, it
> is fine in most cases, but sometimes we have crashk_low_res.
> For example, when "crashkernel=size[KMG],high" combined with
> "crashkernel=size[KMG],low" is used for 64-bit x86.
> 
> Like crashk_res, we introduce the corresponding sysfs file
> "/sys/kernel/kexec_crash_low_size" for crashk_low_res.
> 
> So, the exact total reserved memory is the sum of the two.
> 
> crashk_low_res can also be shrunk via this new interface,
> and users should be aware of what they are doing.

Cc Yinghai Lu for review since he introduced the ,high and ,low logic.

> 
> Suggested-by: Dave Young <dyoung@redhat.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>  include/linux/kexec.h |  4 ++--
>  kernel/kexec_core.c   | 23 ++++++++++++-----------
>  kernel/ksysfs.c       | 25 +++++++++++++++++++++++--
>  3 files changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index d743777..4f271fc 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -304,8 +304,8 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
>  		unsigned long long *crash_size, unsigned long long *crash_base);
>  int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
>  		unsigned long long *crash_size, unsigned long long *crash_base);
> -int crash_shrink_memory(unsigned long new_size);
> -size_t crash_get_memory_size(void);
> +int crash_shrink_memory(struct resource *res, unsigned long new_size);
> +size_t crash_get_memory_size(struct resource *res);
>  void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
>  
>  int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 5616755..707d18e 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -925,13 +925,13 @@ void crash_kexec(struct pt_regs *regs)
>  	}
>  }
>  
> -size_t crash_get_memory_size(void)
> +size_t crash_get_memory_size(struct resource *res)
>  {
>  	size_t size = 0;
>  
>  	mutex_lock(&kexec_mutex);
> -	if (crashk_res.end != crashk_res.start)
> -		size = resource_size(&crashk_res);
> +	if (res->end != res->start)
> +		size = resource_size(res);
>  	mutex_unlock(&kexec_mutex);
>  	return size;
>  }
> @@ -945,7 +945,7 @@ void __weak crash_free_reserved_phys_range(unsigned long begin,
>  		free_reserved_page(boot_pfn_to_page(addr >> PAGE_SHIFT));
>  }
>  
> -int crash_shrink_memory(unsigned long new_size)
> +int crash_shrink_memory(struct resource *res, unsigned long new_size)
>  {
>  	int ret = 0;
>  	unsigned long start, end;
> @@ -958,8 +958,9 @@ int crash_shrink_memory(unsigned long new_size)
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> -	start = crashk_res.start;
> -	end = crashk_res.end;
> +
> +	start = res->start;
> +	end = res->end;
>  	old_size = (end == 0) ? 0 : end - start + 1;
>  	if (new_size >= old_size) {
>  		ret = (new_size == old_size) ? 0 : -EINVAL;
> @@ -975,17 +976,17 @@ int crash_shrink_memory(unsigned long new_size)
>  	start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
>  	end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
>  
> -	crash_free_reserved_phys_range(end, crashk_res.end);
> +	crash_free_reserved_phys_range(end, res->end);
>  
> -	if ((start == end) && (crashk_res.parent != NULL))
> -		release_resource(&crashk_res);
> +	if ((start == end) && (res->parent != NULL))
> +		release_resource(res);
>  
>  	ram_res->start = end;
> -	ram_res->end = crashk_res.end;
> +	ram_res->end = res->end;
>  	ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
>  	ram_res->name = "System RAM";
>  
> -	crashk_res.end = end - 1;
> +	res->end = end - 1;
>  
>  	insert_resource(&iomem_resource, ram_res);
>  
> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> index ee1bc1b..3336fd5 100644
> --- a/kernel/ksysfs.c
> +++ b/kernel/ksysfs.c
> @@ -105,10 +105,30 @@ static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
>  }
>  KERNEL_ATTR_RO(kexec_crash_loaded);
>  
> +static ssize_t kexec_crash_low_size_show(struct kobject *kobj,
> +				       struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%zu\n", crash_get_memory_size(&crashk_low_res));
> +}
> +static ssize_t kexec_crash_low_size_store(struct kobject *kobj,
> +				   struct kobj_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	unsigned long cnt;
> +	int ret;
> +
> +	if (kstrtoul(buf, 0, &cnt))
> +		return -EINVAL;
> +
> +	ret = crash_shrink_memory(&crashk_low_res, cnt);
> +	return ret < 0 ? ret : count;
> +}
> +KERNEL_ATTR_RW(kexec_crash_low_size);
> +
>  static ssize_t kexec_crash_size_show(struct kobject *kobj,
>  				       struct kobj_attribute *attr, char *buf)
>  {
> -	return sprintf(buf, "%zu\n", crash_get_memory_size());
> +	return sprintf(buf, "%zu\n", crash_get_memory_size(&crashk_res));
>  }
>  static ssize_t kexec_crash_size_store(struct kobject *kobj,
>  				   struct kobj_attribute *attr,
> @@ -120,7 +140,7 @@ static ssize_t kexec_crash_size_store(struct kobject *kobj,
>  	if (kstrtoul(buf, 0, &cnt))
>  		return -EINVAL;
>  
> -	ret = crash_shrink_memory(cnt);
> +	ret = crash_shrink_memory(&crashk_res, cnt);
>  	return ret < 0 ? ret : count;
>  }
>  KERNEL_ATTR_RW(kexec_crash_size);
> @@ -218,6 +238,7 @@ static struct attribute * kernel_attrs[] = {
>  #ifdef CONFIG_KEXEC_CORE
>  	&kexec_loaded_attr.attr,
>  	&kexec_crash_loaded_attr.attr,
> +	&kexec_crash_low_size_attr.attr,
>  	&kexec_crash_size_attr.attr,
>  	&vmcoreinfo_attr.attr,
>  #endif
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[Yinghai Lu]

On Wed, Aug 17, 2016 at 1:20 AM, Dave Young <dyoung@redhat.com> wrote:
> On 08/17/16 at 09:50am, Xunlei Pang wrote:
>> "/sys/kernel/kexec_crash_size" only handles crashk_res, it
>> is fine in most cases, but sometimes we have crashk_low_res.
>> For example, when "crashkernel=size[KMG],high" combined with
>> "crashkernel=size[KMG],low" is used for 64-bit x86.
>>
>> Like crashk_res, we introduce the corresponding sysfs file
>> "/sys/kernel/kexec_crash_low_size" for crashk_low_res.
>>
>> So, the exact total reserved memory is the sum of the two.
>>
>> crashk_low_res can also be shrunk via this new interface,
>> and users should be aware of what they are doing.
...
>> @@ -218,6 +238,7 @@ static struct attribute * kernel_attrs[] = {
>>  #ifdef CONFIG_KEXEC_CORE
>>       &kexec_loaded_attr.attr,
>>       &kexec_crash_loaded_attr.attr,
>> +     &kexec_crash_low_size_attr.attr,
>>       &kexec_crash_size_attr.attr,
>>       &vmcoreinfo_attr.attr,
>>  #endif

would be better if you can use attribute_group .is_visible to control showing of
crash_low_size only when the crash_base is above 4G.

Thanks

Yinghai

Re: [PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[Dave Young]

On 08/23/16 at 06:11pm, Yinghai Lu wrote:
> On Wed, Aug 17, 2016 at 1:20 AM, Dave Young <dyoung@redhat.com> wrote:
> > On 08/17/16 at 09:50am, Xunlei Pang wrote:
> >> "/sys/kernel/kexec_crash_size" only handles crashk_res, it
> >> is fine in most cases, but sometimes we have crashk_low_res.
> >> For example, when "crashkernel=size[KMG],high" combined with
> >> "crashkernel=size[KMG],low" is used for 64-bit x86.
> >>
> >> Like crashk_res, we introduce the corresponding sysfs file
> >> "/sys/kernel/kexec_crash_low_size" for crashk_low_res.
> >>
> >> So, the exact total reserved memory is the sum of the two.
> >>
> >> crashk_low_res can also be shrunk via this new interface,
> >> and users should be aware of what they are doing.
> ...
> >> @@ -218,6 +238,7 @@ static struct attribute * kernel_attrs[] = {
> >>  #ifdef CONFIG_KEXEC_CORE
> >>       &kexec_loaded_attr.attr,
> >>       &kexec_crash_loaded_attr.attr,
> >> +     &kexec_crash_low_size_attr.attr,
> >>       &kexec_crash_size_attr.attr,
> >>       &vmcoreinfo_attr.attr,
> >>  #endif
> 
> would be better if you can use attribute_group .is_visible to control showing of
> crash_low_size only when the crash_base is above 4G.

I have same feeling that it looks odd to show low in sysfs in case no
crashkernel=,high being used. Even if crashkernel=,high is used only in
x86 the resource crashk_low is in common code. What do you think to move
it to x86?

Thanks
Dave

> 
> Thanks
> 
> Yinghai
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 1/2] kexec: Introduce "/sys/kernel/kexec_crash_low_size"

[Xunlei Pang]

On 2016/08/24 at 16:20, Dave Young wrote:
> On 08/23/16 at 06:11pm, Yinghai Lu wrote:
>> On Wed, Aug 17, 2016 at 1:20 AM, Dave Young <dyoung@redhat.com> wrote:
>>> On 08/17/16 at 09:50am, Xunlei Pang wrote:
>>>> "/sys/kernel/kexec_crash_size" only handles crashk_res, it
>>>> is fine in most cases, but sometimes we have crashk_low_res.
>>>> For example, when "crashkernel=size[KMG],high" combined with
>>>> "crashkernel=size[KMG],low" is used for 64-bit x86.
>>>>
>>>> Like crashk_res, we introduce the corresponding sysfs file
>>>> "/sys/kernel/kexec_crash_low_size" for crashk_low_res.
>>>>
>>>> So, the exact total reserved memory is the sum of the two.
>>>>
>>>> crashk_low_res can also be shrunk via this new interface,
>>>> and users should be aware of what they are doing.
>> ...
>>>> @@ -218,6 +238,7 @@ static struct attribute * kernel_attrs[] = {
>>>>  #ifdef CONFIG_KEXEC_CORE
>>>>       &kexec_loaded_attr.attr,
>>>>       &kexec_crash_loaded_attr.attr,
>>>> +     &kexec_crash_low_size_attr.attr,
>>>>       &kexec_crash_size_attr.attr,
>>>>       &vmcoreinfo_attr.attr,
>>>>  #endif
>> would be better if you can use attribute_group .is_visible to control showing of
>> crash_low_size only when the crash_base is above 4G.
> I have same feeling that it looks odd to show low in sysfs in case no
> crashkernel=,high being used. Even if crashkernel=,high is used only in
> x86 the resource crashk_low is in common code. What do you think to move
> it to x86?

If want to put some restriction on it, I'd prefer to move crashk_low to arch x86, to make
it x86-specific.

We can show the interface unconditionally. If it isn't used, its size is 0, it doesn't matter.

Regards,
Xunlei

>
> Thanks
> Dave
>
>> Thanks
>>
>> Yinghai
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec