Re: [Xen-devel] [PATCH v3 3/9] mm: Scrub pages in alloc_heap_pages() if needed

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 05/04/2017 10:44 AM, Jan Beulich wrote:
>>>> On 14.04.17 at 17:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> When allocating pages in alloc_heap_pages() first look for clean pages.
> As expressed before, there are cases when we don't really need
> scrubbed pages. Hence the local variable "use_unscrubbed" below
> should really be some form of input to alloc_heap_pages().

That would be alloc_xenheap_pages() only, in which case can I just
initialize the still local use_unscrubbed
as
    use_unscrubbed = (zone_lo == MEMZONE_XEN)

Or do you prefer this to be explicit?

>
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -700,34 +700,17 @@ static struct page_info *alloc_heap_pages(
>>      unsigned int order, unsigned int memflags,
>>      struct domain *d)
>>  {
>> -    unsigned int i, j, zone = 0, nodemask_retry = 0;
>> -    nodeid_t first_node, node = MEMF_get_node(memflags), req_node = node;
>> +    unsigned int i, j, zone, nodemask_retry;
>> +    nodeid_t first_node, node, req_node;
>>      unsigned long request = 1UL << order;
>>      struct page_info *pg;
>> -    nodemask_t nodemask = (d != NULL ) ? d->node_affinity : node_online_map;
>> -    bool_t need_tlbflush = 0;
>> +    nodemask_t nodemask;
>> +    bool need_scrub, need_tlbflush = false, use_unscrubbed = false;
>>      uint32_t tlbflush_timestamp = 0;
>>  
>>      /* Make sure there are enough bits in memflags for nodeID. */
>>      BUILD_BUG_ON((_MEMF_bits - _MEMF_node) < (8 * sizeof(nodeid_t)));
>>  
>> -    if ( node == NUMA_NO_NODE )
>> -    {
>> -        if ( d != NULL )
>> -        {
>> -            node = next_node(d->last_alloc_node, nodemask);
>> -            if ( node >= MAX_NUMNODES )
>> -                node = first_node(nodemask);
>> -        }
>> -        if ( node >= MAX_NUMNODES )
>> -            node = cpu_to_node(smp_processor_id());
>> -    }
>> -    first_node = node;
>> -
>> -    ASSERT(node < MAX_NUMNODES);
>> -    ASSERT(zone_lo <= zone_hi);
>> -    ASSERT(zone_hi < NR_ZONES);
> The last two can remain where they are (but see also below).
>
>> @@ -754,6 +740,28 @@ static struct page_info *alloc_heap_pages(
>>           tmem_freeable_pages() )
>>          goto try_tmem;
>>  
>> + again:
> Is there any hope to get away without such an ugly pseudo loop?
> E.g. by making this function a helper function of a relatively thin
> wrapper named alloc_heap_pages(), invoking this helper twice
> unless use_unscrubbed is true (as said, this ought to be an input)?


OK, let me see if I can beautify this.


-boris

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