lkml.org 
[lkml]   [2017]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Xen-devel] [PATCH v2] xen/balloon: Mark unallocated host memory as UNUSABLE
>>> On 19.12.17 at 15:25, <boris.ostrovsky@oracle.com> wrote:
> On 12/19/2017 03:23 AM, Jan Beulich wrote:
>>>>> On 18.12.17 at 23:22, <boris.ostrovsky@oracle.com> wrote:
>>> + if (!xen_e820_table)
>>> + return;
>> Not saying "out of memory" here is certainly fine, but shouldn't
>> there nevertheless be a warning, as failure to go through the
>> rest of the function will impact overall functionality?
>
> Commit ebfdc40969f claims that these types of messages are unnecessary
> because allocation failures are signalled by the memory subsystem.

But the memory subsystem can't possibly provide an indication of
what will not work because of the failed allocation.

>>> + memmap.nr_entries = ARRAY_SIZE(xen_e820_table->entries);
>> Is it really reasonable to have a static upper bound here? As we
>> know especially EFI systems can come with a pretty scattered
>> (pseudo) E820 table. Even if (iirc) this has a static upper bound
>> right now in the hypervisor too, it would be nice if the kernel
>> didn't need further changes once the hypervisor is being made
>> more flexible.
>
> This is how we obtain the map in xen_memory_setup(). Are you suggesting
> that we should query for the size first?

That would be better, I think.

>>> + /* Mark non-RAM regions as not available. */
>>> + for (; i < memmap.nr_entries; i++) {
>>> + entry = &xen_e820_table->entries[i];
>>> +
>>> + if (entry->type == E820_TYPE_RAM)
>>> + continue;
>> I can't seem to match up this with ...
>>
>>> + if (entry->addr >= hostmem_resource->end)
>>> + break;
>>> +
>>> + res = kzalloc(sizeof(*res), GFP_KERNEL);
>>> + if (!res)
>>> + goto out;
>>> +
>>> + res->name = "Host memory";
>> ... this. Do you mean != instead (with the comment ahead of the
>> loop also clarified, saying something like "host RAM regions which
>> aren't RAM for us")? And perhaps better "Host RAM"?
>
> Right, this is not memory but rather something else (and so "!=" is
> correct). "Unavailable host RAM"?

If you like to be even more specific than what I had suggested -
sure.

Jan

\
 
 \ /
  Last update: 2017-12-19 15:40    [W:0.068 / U:0.616 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site