From: Xunlei Pang <xlpang@redhat.com>
To: Dave Young <dyoung@redhat.com>
Cc: akpm@linux-foundation.org, kexec@lists.infradead.org,
linux-kernel@vger.kernel.org,
Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH 1/2] kexec: Set KEXEC_TYPE_CRASH before sanity_check_segment_list()
Date: Wed, 2 Dec 2015 14:03:47 +0800 [thread overview]
Message-ID: <565E89C3.40608@redhat.com> (raw)
In-Reply-To: <20151202054745.GA2899@dhcp-128-65.nay.redhat.com>
Hi Dave,
On 12/02/2015 at 01:47 PM, Dave Young wrote:
> Ccing Andrew since usually he monitors and picks up kexec patches.
>
> On 12/02/15 at 01:26pm, Xunlei Pang wrote:
>> sanity_check_segment_list() checks KEXEC_TYPE_CRASH flag to ensure
>> all the segments of the loaded crash kernel are winthin the kernel
>> crash resource limits, so set the flag beforehand.
> Looks good except a nitpick, see comments inline.
>
> Otherwise:
> Acked-by: Dave Young <dyoung@redhat.com>
>
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>> ---
>> kernel/kexec.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>> index d873b64..9624391 100644
>> --- a/kernel/kexec.c
>> +++ b/kernel/kexec.c
>> @@ -63,16 +63,19 @@ static int kimage_alloc_init(struct kimage **rimage, unsigned long entry,
>> if (ret)
>> goto out_free_image;
>>
>> - ret = sanity_check_segment_list(image);
>> - if (ret)
>> - goto out_free_image;
>> -
>> - /* Enable the special crash kernel control page allocation policy. */
>> + /*
>> + * Enable the special crash kernel control page allocation policy,
>> + * and set the crash type flag.
> It is obvious it is setting the crash type flag, so no need to add extra
> comment.
>
>> + */
>> if (kexec_on_panic) {
> Like kexec_file.c, move /* Enable ... policy */ here sounds better.
Yea, this is better. Will do, thanks.
Regards,
Xunlei
>
>> image->control_page = crashk_res.start;
>> image->type = KEXEC_TYPE_CRASH;
>> }
>>
>> + ret = sanity_check_segment_list(image);
>> + if (ret)
>> + goto out_free_image;
>> +
>> /*
>> * Find a location for the control code buffer, and add it
>> * the vector of segments so that it's pages will also be
>> --
>> 2.5.0
>>
> Thanks
> Dave
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Xunlei Pang <xlpang@redhat.com>
To: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
Eric Biederman <ebiederm@xmission.com>,
akpm@linux-foundation.org
Subject: Re: [PATCH 1/2] kexec: Set KEXEC_TYPE_CRASH before sanity_check_segment_list()
Date: Wed, 2 Dec 2015 14:03:47 +0800 [thread overview]
Message-ID: <565E89C3.40608@redhat.com> (raw)
In-Reply-To: <20151202054745.GA2899@dhcp-128-65.nay.redhat.com>
Hi Dave,
On 12/02/2015 at 01:47 PM, Dave Young wrote:
> Ccing Andrew since usually he monitors and picks up kexec patches.
>
> On 12/02/15 at 01:26pm, Xunlei Pang wrote:
>> sanity_check_segment_list() checks KEXEC_TYPE_CRASH flag to ensure
>> all the segments of the loaded crash kernel are winthin the kernel
>> crash resource limits, so set the flag beforehand.
> Looks good except a nitpick, see comments inline.
>
> Otherwise:
> Acked-by: Dave Young <dyoung@redhat.com>
>
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>> ---
>> kernel/kexec.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>> index d873b64..9624391 100644
>> --- a/kernel/kexec.c
>> +++ b/kernel/kexec.c
>> @@ -63,16 +63,19 @@ static int kimage_alloc_init(struct kimage **rimage, unsigned long entry,
>> if (ret)
>> goto out_free_image;
>>
>> - ret = sanity_check_segment_list(image);
>> - if (ret)
>> - goto out_free_image;
>> -
>> - /* Enable the special crash kernel control page allocation policy. */
>> + /*
>> + * Enable the special crash kernel control page allocation policy,
>> + * and set the crash type flag.
> It is obvious it is setting the crash type flag, so no need to add extra
> comment.
>
>> + */
>> if (kexec_on_panic) {
> Like kexec_file.c, move /* Enable ... policy */ here sounds better.
Yea, this is better. Will do, thanks.
Regards,
Xunlei
>
>> image->control_page = crashk_res.start;
>> image->type = KEXEC_TYPE_CRASH;
>> }
>>
>> + ret = sanity_check_segment_list(image);
>> + if (ret)
>> + goto out_free_image;
>> +
>> /*
>> * Find a location for the control code buffer, and add it
>> * the vector of segments so that it's pages will also be
>> --
>> 2.5.0
>>
> Thanks
> Dave
next prev parent reply other threads:[~2015-12-02 6:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-02 5:26 [PATCH 1/2] kexec: Set KEXEC_TYPE_CRASH before sanity_check_segment_list() Xunlei Pang
2015-12-02 5:26 ` Xunlei Pang
2015-12-02 5:47 ` Dave Young
2015-12-02 5:47 ` Dave Young
2015-12-02 6:03 ` Xunlei Pang [this message]
2015-12-02 6:03 ` Xunlei Pang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=565E89C3.40608@redhat.com \
--to=xlpang@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dyoung@redhat.com \
--cc=ebiederm@xmission.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.