[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <57B177DA.3030005@redhat.com>
Date: Mon, 15 Aug 2016 16:05:46 +0800
From: Xunlei Pang <xpang@...hat.com>
To: Dave Young <dyoung@...hat.com>, Xunlei Pang <xlpang@...hat.com>
Cc: Baoquan He <bhe@...hat.com>, kexec@...ts.infradead.org,
linux-kernel@...r.kernel.org, ebiederm@...ssion.com,
akpm@...ux-foundation.org, Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH] kexec: Account crashk_low_res to kexec_crash_size
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@...hat.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@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
> _______________________________________________
> kexec mailing list
> kexec@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Powered by blists - more mailing lists