xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH] kexec: allow relaxed placement specification via command line
Date: Tue, 31 May 2016 17:02:06 +0100	[thread overview]
Message-ID: <574DB57E.1000606@citrix.com> (raw)
In-Reply-To: <574DA36202000078000F0095@prv-mh.provo.novell.com>

On 31/05/16 13:44, Jan Beulich wrote:
>>>> On 31.05.16 at 12:30, <david.vrabel@citrix.com> wrote:
>> On 30/05/16 14:48, Jan Beulich wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -1044,13 +1044,19 @@ void __init noreturn __start_xen(unsigne
>>>          }
>>>  
>>>  #ifdef CONFIG_KEXEC
>>> -        /* Don't overlap with modules. */
>>> -        e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
>>> -                             mod, mbi->mods_count, -1);
>>> -        if ( !kexec_crash_area.start && (s < e) )
>>
>> I think we want a comment here.
>>
>> /*
>>  * Looking backwards from the crash area limit, find a large enough
>>  * crash area that does not overlap with modules.
>>  */
> 
> Sure, added.
> 
>>> +        while ( !kexec_crash_area.start )
>>
>> Does this mean that if an @<offset> is specified we no longer check for
>> overlapping modules?
> 
> We didn't do any more checking before. If you look at the old
> code above, we called consider_modules() only to possibly alter
> e. All the rest of the old code was similarly dependent upon
> !kexec_crash_area.start. That other case is being taken care
> of earlier anyway - see kexec_reserve_area()'s first invocation.
> 
> But yes, it looks like there's an overlap check missing there (iiuc
> relevant really only for the initrd, as that's the only thing the
> memory of which may not get copied but simply directly handed
> to Dom0).

Ok.  Any additional improvement can be done later so if you add the comment,

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-05-31 16:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30 13:48 [PATCH] kexec: allow relaxed placement specification via command line Jan Beulich
2016-05-31 10:24 ` Andrew Cooper
2016-05-31 10:50   ` Jan Beulich
2016-05-31 10:30 ` David Vrabel
2016-05-31 12:44   ` Jan Beulich
2016-05-31 16:02     ` David Vrabel [this message]
2016-06-01 10:26 ` Daniel Kiper
2016-06-01 10:42   ` Jan Beulich

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=574DB57E.1000606@citrix.com \
    --to=david.vrabel@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=daniel.kiper@oracle.com \
    --cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).