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

2016-08-24 Thread 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  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


___
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"

2016-08-24 Thread Dave Young 
On 08/23/16 at 06:11pm, Yinghai Lu wrote:
> On Wed, Aug 17, 2016 at 1:20 AM, Dave Young  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

___
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"

2016-08-23 Thread Yinghai Lu 
On Wed, Aug 17, 2016 at 1:20 AM, Dave Young  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

___
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"

2016-08-17 Thread 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 
> Signed-off-by: Xunlei Pang 
> ---
>  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);
> +

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

2016-08-16 Thread 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 
Signed-off-by: Xunlei Pang 
---
 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));