[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <57B415A5.60104@redhat.com>
Date: Wed, 17 Aug 2016 15:43:33 +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 v2 2/2] kexec: Consider crashk_low_res in
sanity_check_segment_list()
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@...hat.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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Powered by blists - more mailing lists