Re: [Xen-devel] [PATCH] xen/memguard: Drop memguard_init() entirely

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.02.16 at 11:29, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/02/16 10:02, Jan Beulich wrote:
>>>>> On 19.02.16 at 17:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 19/02/16 14:44, Jan Beulich wrote:
>>>>>>> On 18.02.16 at 19:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> It is not obvious what this code is doing.  Most of it dates from 
>>>>> 2007/2008,
>>>>> and there have been substantial changes in Xen's memory handling since 
>>>>> then.
>>>> Deleting code which isn't understood what it is or was once used
>>>> for is sub-optimal.
>>>>
>>>>> It was previously optional, and isn't needed for any of the memguard
>>>>> infrastructure to function.  The use of MAP_SMALL_PAGES causes needless
>>>>> shattering of superpages.
>>>> Perhaps that's what is its purpose? Let's ask Keir, whom you didn't
>>>> even Cc.
>>> I don't see this patch being different in nature to your "x86: drop
>>> failsafe callback invocation from assembly".
>> That other patch explains why the code is (and never was)
>> necessary, whereas you just guess.
>>
>>> As I explain in the second paragraph, these calls are strictly optional,
>>> as they are omitted for release builds.  They also have no impact on the
>>> rest of the memguard infrastructure to function, as
>>> __memguard_change_range() also uses map_pages_to_xen().
>>>
>>> So despite not being sure why it is like it is, I am stating with that
>>> it is not needed with Xen in its current form.
>> I actually think the reason is to avoid the memory allocation which
>> might result the first time a 2M page gets split up, as that memory
>> allocation might fail (which nowadays gets a proper -ENOMEM
>> communicated out of map_pages_to_xen(), but that hasn't been
>> the case in the early days).
> 
> And isn't the case anywhere in the memguard infrastructure.
> 
> At the end of this series, there is no shattering of any superpages in
> the normal .text/data/bss region, but there is guarding of the pcpu
> stack which is liable to shatter specific superpages mapping the
> directmap region.

In which case I guess I'm fine with the change, but I'd like you to
re-word the commit message accordingly (in particular make it less
vague).

Jan


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