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 at redhat.com> > >> Signed-off-by: Xunlei Pang <xlpang at 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