Re: [PATCH v4 1/6] xen: do not free reserved memory into heap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.05.2022 04:29, Penny Zheng wrote:
> Hi Jan and Julien
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Wednesday, May 18, 2022 12:11 AM
>> To: Penny Zheng <Penny.Zheng@xxxxxxx>
>> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>;
>> Julien Grall <julien@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; 
>> Wei
>> Liu <wl@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v4 1/6] xen: do not free reserved memory into heap
>>
>> On 10.05.2022 04:27, Penny Zheng wrote:
>>> @@ -2762,6 +2767,12 @@ int __init acquire_domstatic_pages(struct
>>> domain *d, mfn_t smfn,
>>>
>>>      return 0;
>>>  }
>>> +#else
>>> +void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
>>> +                          bool need_scrub) {
>>> +    ASSERT_UNREACHABLE();
>>> +}
>>>  #endif
>>
>> As before I do not agree that we need this (or similar) stub functions. As
>> already suggested I think that instead Arm wants to #define PGC_reserved (to
>> non-zero) only when !CONFIG_STATIC_MEMORY, just like is already the case
>> on x86.
>>
> 
> Ok, if you do not like the stub function, then what about I putting the 
> #ifdef-array back
> to the common where free_staticmem_pages is used:
> #ifdef CONFIG_STATIC_MEMORY
>       if ( pg->count_info & PGC_reserved )
>           /* Reserved page shall not go back to the heap. */
>           return free_staticmem_pages(pg, 1UL << order, need_scrub);
> #endif

No. Stub functions, when they are really needed, are generally preferable
over #ifdef-ary inside functions (for readability reasons at least). Yet
even better is if stub functions can be avoided altogether.

Jan