All of lore.kernel.org
 help / color / mirror / Atom feed
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


  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.