public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kexec: Account crashk_low_res to kexec_crash_size
@ 2016-08-13  8:26 Xunlei Pang
  2016-08-15  7:17 ` Dave Young
  0 siblings, 1 reply; 4+ messages in thread
From: Xunlei Pang @ 2016-08-13  8:26 UTC (permalink / raw)
  To: linux-kernel, kexec
  Cc: akpm, ebiederm, Vivek Goyal, Dave Young, Baoquan He, Xunlei Pang

"/sys/kernel/kexec_crash_size" only includes 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.

Let "/sys/kernel/kexec_crash_size" reflect all the reserved
memory including crashk_low_res, this is more understandable
from its naming.

Although we can get all the crash memory from "/proc/iomem"
by filtering all "Crash kernel" keyword, it is more convenient
to utilize this file, and the two ways should stay consistent.

Note that write to "/sys/kernel/kexec_crash_size" is to shrink
the reserved memory, and we want to shrink crashk_res only.
So we add some additional check in crash_shrink_memory() since
crashk_low_res now is involved.

Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
 kernel/kexec_core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..d5ae780 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -932,6 +932,8 @@ size_t crash_get_memory_size(void)
 	mutex_lock(&kexec_mutex);
 	if (crashk_res.end != crashk_res.start)
 		size = resource_size(&crashk_res);
+	if (crashk_low_res.end != crashk_low_res.start)
+		size += resource_size(&crashk_low_res);
 	mutex_unlock(&kexec_mutex);
 	return size;
 }
@@ -949,7 +951,7 @@ int crash_shrink_memory(unsigned long new_size)
 {
 	int ret = 0;
 	unsigned long start, end;
-	unsigned long old_size;
+	unsigned long low_size, old_size;
 	struct resource *ram_res;
 
 	mutex_lock(&kexec_mutex);
@@ -958,6 +960,17 @@ int crash_shrink_memory(unsigned long new_size)
 		ret = -ENOENT;
 		goto unlock;
 	}
+
+	start = crashk_low_res.start;
+	end = crashk_low_res.end;
+	low_size = (end == 0) ? 0 : end - start + 1;
+	/* Do not shrink crashk_low_res. */
+	if (new_size <= low_size) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	new_size -= low_size;
 	start = crashk_res.start;
 	end = crashk_res.end;
 	old_size = (end == 0) ? 0 : end - start + 1;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] kexec: Account crashk_low_res to kexec_crash_size
  2016-08-13  8:26 [PATCH] kexec: Account crashk_low_res to kexec_crash_size Xunlei Pang
@ 2016-08-15  7:17 ` Dave Young
  2016-08-15  8:05   ` Xunlei Pang
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Young @ 2016-08-15  7:17 UTC (permalink / raw)
  To: Xunlei Pang; +Cc: linux-kernel, kexec, Baoquan He, ebiederm, akpm, Vivek Goyal

Hi Xunlei,

On 08/13/16 at 04:26pm, Xunlei Pang wrote:
> "/sys/kernel/kexec_crash_size" only includes 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.
> 
> Let "/sys/kernel/kexec_crash_size" reflect all the reserved
> memory including crashk_low_res, this is more understandable
> from its naming.

Maybe export another file for the kexec_crash_low_size so that
we can clearly get how much the low area is.

> 
> Although we can get all the crash memory from "/proc/iomem"
> by filtering all "Crash kernel" keyword, it is more convenient
> to utilize this file, and the two ways should stay consistent.

Shrink low area does not make much sense, one may either use it or
shrink it to 0.

Actually think more about it, the crashk_low is only for x86,
it might be even better to move it to x86 code instead of in
common code.

Opinion?

Thanks
Dave
> 
> Note that write to "/sys/kernel/kexec_crash_size" is to shrink
> the reserved memory, and we want to shrink crashk_res only.
> So we add some additional check in crash_shrink_memory() since
> crashk_low_res now is involved.
> 
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>  kernel/kexec_core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 5616755..d5ae780 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -932,6 +932,8 @@ size_t crash_get_memory_size(void)
>  	mutex_lock(&kexec_mutex);
>  	if (crashk_res.end != crashk_res.start)
>  		size = resource_size(&crashk_res);
> +	if (crashk_low_res.end != crashk_low_res.start)
> +		size += resource_size(&crashk_low_res);
>  	mutex_unlock(&kexec_mutex);
>  	return size;
>  }
> @@ -949,7 +951,7 @@ int crash_shrink_memory(unsigned long new_size)
>  {
>  	int ret = 0;
>  	unsigned long start, end;
> -	unsigned long old_size;
> +	unsigned long low_size, old_size;
>  	struct resource *ram_res;
>  
>  	mutex_lock(&kexec_mutex);
> @@ -958,6 +960,17 @@ int crash_shrink_memory(unsigned long new_size)
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> +
> +	start = crashk_low_res.start;
> +	end = crashk_low_res.end;
> +	low_size = (end == 0) ? 0 : end - start + 1;
> +	/* Do not shrink crashk_low_res. */
> +	if (new_size <= low_size) {
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	new_size -= low_size;
>  	start = crashk_res.start;
>  	end = crashk_res.end;
>  	old_size = (end == 0) ? 0 : end - start + 1;
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] kexec: Account crashk_low_res to kexec_crash_size
  2016-08-15  7:17 ` Dave Young
@ 2016-08-15  8:05   ` Xunlei Pang
  2016-08-16 10:42     ` Dave Young
  0 siblings, 1 reply; 4+ messages in thread
From: Xunlei Pang @ 2016-08-15  8:05 UTC (permalink / raw)
  To: Dave Young, Xunlei Pang
  Cc: Baoquan He, kexec, linux-kernel, ebiederm, akpm, Vivek Goyal

On 2016/08/15 at 15:17, Dave Young wrote:
> Hi Xunlei,
>
> On 08/13/16 at 04:26pm, Xunlei Pang wrote:
>> "/sys/kernel/kexec_crash_size" only includes 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.
>>
>> Let "/sys/kernel/kexec_crash_size" reflect all the reserved
>> memory including crashk_low_res, this is more understandable
>> from its naming.
> Maybe export another file for the kexec_crash_low_size so that
> we can clearly get how much the low area is.

I'm fine with it.

>> Although we can get all the crash memory from "/proc/iomem"
>> by filtering all "Crash kernel" keyword, it is more convenient
>> to utilize this file, and the two ways should stay consistent.
> Shrink low area does not make much sense, one may either use it or
> shrink it to 0.
>
> Actually think more about it, the crashk_low is only for x86,
> it might be even better to move it to x86 code instead of in
> common code.
>
> Opinion?

crashk_low is defined in kernel/kexec_core.c, it's an architecture independent definition
though it's only used by x86 currently, maybe it can be used by others in the future.
It's why I'm not handling it specifically for x86.

I just tested the original proc interface further, and it can be shrinked to be zero.
So I guess we can ease the restriction on shrinking the low area as well.

What do you think?

Regards,
Xunlei

>
> Thanks
> Dave
>> Note that write to "/sys/kernel/kexec_crash_size" is to shrink
>> the reserved memory, and we want to shrink crashk_res only.
>> So we add some additional check in crash_shrink_memory() since
>> crashk_low_res now is involved.
>>
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>> ---
>>  kernel/kexec_core.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index 5616755..d5ae780 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -932,6 +932,8 @@ size_t crash_get_memory_size(void)
>>  	mutex_lock(&kexec_mutex);
>>  	if (crashk_res.end != crashk_res.start)
>>  		size = resource_size(&crashk_res);
>> +	if (crashk_low_res.end != crashk_low_res.start)
>> +		size += resource_size(&crashk_low_res);
>>  	mutex_unlock(&kexec_mutex);
>>  	return size;
>>  }
>> @@ -949,7 +951,7 @@ int crash_shrink_memory(unsigned long new_size)
>>  {
>>  	int ret = 0;
>>  	unsigned long start, end;
>> -	unsigned long old_size;
>> +	unsigned long low_size, old_size;
>>  	struct resource *ram_res;
>>  
>>  	mutex_lock(&kexec_mutex);
>> @@ -958,6 +960,17 @@ int crash_shrink_memory(unsigned long new_size)
>>  		ret = -ENOENT;
>>  		goto unlock;
>>  	}
>> +
>> +	start = crashk_low_res.start;
>> +	end = crashk_low_res.end;
>> +	low_size = (end == 0) ? 0 : end - start + 1;
>> +	/* Do not shrink crashk_low_res. */
>> +	if (new_size <= low_size) {
>> +		ret = -EINVAL;
>> +		goto unlock;
>> +	}
>> +
>> +	new_size -= low_size;
>>  	start = crashk_res.start;
>>  	end = crashk_res.end;
>>  	old_size = (end == 0) ? 0 : end - start + 1;
>> -- 
>> 1.8.3.1
>>
>>
>> _______________________________________________
>> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] kexec: Account crashk_low_res to kexec_crash_size
  2016-08-15  8:05   ` Xunlei Pang
@ 2016-08-16 10:42     ` Dave Young
  0 siblings, 0 replies; 4+ messages in thread
From: Dave Young @ 2016-08-16 10:42 UTC (permalink / raw)
  To: xlpang; +Cc: Baoquan He, kexec, linux-kernel, ebiederm, akpm, Vivek Goyal

Hi,

On 08/15/16 at 04:05pm, Xunlei Pang wrote:
> On 2016/08/15 at 15:17, Dave Young wrote:
> > Hi Xunlei,
> >
> > On 08/13/16 at 04:26pm, Xunlei Pang wrote:
> >> "/sys/kernel/kexec_crash_size" only includes 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.
> >>
> >> Let "/sys/kernel/kexec_crash_size" reflect all the reserved
> >> memory including crashk_low_res, this is more understandable
> >> from its naming.
> > Maybe export another file for the kexec_crash_low_size so that
> > we can clearly get how much the low area is.
> 
> I'm fine with it.
> 
> >> Although we can get all the crash memory from "/proc/iomem"
> >> by filtering all "Crash kernel" keyword, it is more convenient
> >> to utilize this file, and the two ways should stay consistent.
> > Shrink low area does not make much sense, one may either use it or
> > shrink it to 0.
> >
> > Actually think more about it, the crashk_low is only for x86,
> > it might be even better to move it to x86 code instead of in
> > common code.
> >
> > Opinion?
> 
> crashk_low is defined in kernel/kexec_core.c, it's an architecture independent definition
> though it's only used by x86 currently, maybe it can be used by others in the future.
> It's why I'm not handling it specifically for x86.

Ok, we can leave with it since it is in common code from the very
beginning but I doubt that any other arches will use it.

> 
> I just tested the original proc interface further, and it can be shrinked to be zero.
> So I guess we can ease the restriction on shrinking the low area as well.
> 
> What do you think?

Ok, agreed.

Thanks
Dave

> 
> Regards,
> Xunlei
> 
> >
> > Thanks
> > Dave
> >> Note that write to "/sys/kernel/kexec_crash_size" is to shrink
> >> the reserved memory, and we want to shrink crashk_res only.
> >> So we add some additional check in crash_shrink_memory() since
> >> crashk_low_res now is involved.
> >>
> >> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> >> ---
> >>  kernel/kexec_core.c | 15 ++++++++++++++-
> >>  1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> >> index 5616755..d5ae780 100644
> >> --- a/kernel/kexec_core.c
> >> +++ b/kernel/kexec_core.c
> >> @@ -932,6 +932,8 @@ size_t crash_get_memory_size(void)
> >>  	mutex_lock(&kexec_mutex);
> >>  	if (crashk_res.end != crashk_res.start)
> >>  		size = resource_size(&crashk_res);
> >> +	if (crashk_low_res.end != crashk_low_res.start)
> >> +		size += resource_size(&crashk_low_res);
> >>  	mutex_unlock(&kexec_mutex);
> >>  	return size;
> >>  }
> >> @@ -949,7 +951,7 @@ int crash_shrink_memory(unsigned long new_size)
> >>  {
> >>  	int ret = 0;
> >>  	unsigned long start, end;
> >> -	unsigned long old_size;
> >> +	unsigned long low_size, old_size;
> >>  	struct resource *ram_res;
> >>  
> >>  	mutex_lock(&kexec_mutex);
> >> @@ -958,6 +960,17 @@ int crash_shrink_memory(unsigned long new_size)
> >>  		ret = -ENOENT;
> >>  		goto unlock;
> >>  	}
> >> +
> >> +	start = crashk_low_res.start;
> >> +	end = crashk_low_res.end;
> >> +	low_size = (end == 0) ? 0 : end - start + 1;
> >> +	/* Do not shrink crashk_low_res. */
> >> +	if (new_size <= low_size) {
> >> +		ret = -EINVAL;
> >> +		goto unlock;
> >> +	}
> >> +
> >> +	new_size -= low_size;
> >>  	start = crashk_res.start;
> >>  	end = crashk_res.end;
> >>  	old_size = (end == 0) ? 0 : end - start + 1;
> >> -- 
> >> 1.8.3.1
> >>
> >>
> >> _______________________________________________
> >> 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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-16 10:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-13  8:26 [PATCH] kexec: Account crashk_low_res to kexec_crash_size Xunlei Pang
2016-08-15  7:17 ` Dave Young
2016-08-15  8:05   ` Xunlei Pang
2016-08-16 10:42     ` Dave Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox