Re: [Xen-devel] [PATCH v5 4/8] mm: introduce a helper to get the memory type of a page

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.18 at 18:37, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Aug 17, 2018 at 04:31:21AM -0600, Jan Beulich wrote:
>> >>> On 17.08.18 at 12:17, <JBeulich@xxxxxxxx> wrote:
>> >>>> On 14.08.18 at 15:43, <roger.pau@xxxxxxxxxx> wrote:
>> >> +            switch ( e820.map[i].type )
>> >> +            {
>> >> +            case E820_RAM:
>> >> +                return RAM_TYPE_CONVENTIONAL;
>> >> +
>> >> +            case E820_RESERVED:
>> >> +                return RAM_TYPE_RESERVED;
>> >> +
>> >> +            case E820_UNUSABLE:
>> >> +                return RAM_TYPE_UNUSABLE;
>> >> +
>> >> +            case E820_ACPI:
>> >> +            case E820_NVS:
>> >> +                return RAM_TYPE_ACPI;
>> >> +
>> >> +            default:
>> >> +                ASSERT_UNREACHABLE();
>> >> +                return -1;
>> >> +            }
>> >> +    }
>> >> +
>> >> +    return -1;
>> >> +}
>> > 
>> > One more case to consider: What about a page part of which is
>> > a given type, and the other part of which is simply missing from
>> > the E820 table? I'm uncertain whether in that case it might be a
>> > good idea in general to report it as having the given type; for the
>> > specific purpose you want the function for, that would imo be
>> > quite helpful.
>> 
>> Considering RAM_TYPE_* are bit masks - perhaps the function
>> should OR together all types found for the requested page, and
>> let the caller go from there? And to account for my earlier remark,
>> add a separate RAM_TYPE_UNKNOWN (and never have the function
>> return -1 or some such; its return type then would better be
>> unsigned int)?
> 
> I can do something along this lines, but then the functionality of the
> existing inclusive option is likely to change on boxes having memory
> types not aligned to a page boundary.

That's an expected consequence, yes, and given this is a workaround
option anyway I'm afraid we'll have to live with it being unclear whether
this would be a change to the better or to the worse.

> Also, we should likely discuss what to do with a page that for example
> has both unusable and reserved regions. I would map it in the
> inclusive case because it has a reserved region, but would like to
> hear your opinion.

Not page-aligned unusable regions are pretty suspicious imo. Along
the lines of the above response, I think I'd be fine going either of the
two possible routes - we simply can't tell ahead of time which one
might be better.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel