Re: [PATCH v7 5/7] xen: re-define assign_pages and introduce a new function assign_page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.09.2021 18:23, Stefano Stabellini wrote:
> On Fri, 10 Sep 2021, Jan Beulich wrote:
>> On 10.09.2021 04:52, Penny Zheng wrote:
>>> In order to deal with the trouble of count-to-order conversion when page 
>>> number
>>> is not in a power-of-two, this commit re-define assign_pages for nr pages 
>>> and
>>> assign_page for original page with a single order.
>>>
>>> Backporting confusion could be helped by altering the order of assign_pages
>>> parameters, such that the compiler would point out that adjustments at call
>>> sites are needed.
>>
>> Thanks, this now takes care of my primary concern. However, I (now?) have
>> another (I thought I would have mentioned this before):
>>
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2259,9 +2259,9 @@ void init_domheap_pages(paddr_t ps, paddr_t pe)
>>>  
>>>  
>>>  int assign_pages(
>>> -    struct domain *d,
>>>      struct page_info *pg,
>>> -    unsigned int order,
>>> +    unsigned long nr,
>>
>> If this really is to be "unsigned long" (and not "unsigned int"), then...
> 
> Thanks for spotting this. I think it makes sense to use "unsigned int
> nr" here.

I see you've made the change while committing, but the subsequent patch
then would have needed adjustment as well: It's now silently truncating
an "unsigned long" value to "unsigned int". It was the splitting that's
now needed there _anyway_ that made me wonder whether the patch here
really is worthwhile to have. But of course acquire_domstatic_pages()
could for now also simply reject too large values ...

Jan