Re: [EXTERNAL] Re: [PATCH v2] core: Fix aligned_start calculation in page_alloc_internal()

['Pratyush Yadav' via Jailhouse]

On 01/07/19 1:05 PM, Jan Kiszka wrote:
> On 01.07.19 09:29, Pratyush Yadav wrote:
>> Right now, page_alloc_aligned() can fail to give aligned pages when more
>> than one page is being allocated. This is because the aligned_start
>> calculation is flawed.
>>
>> Taking an example from a test case, let's say 8 pages need to be
>> allocated. This means an alignment of 15 bits. The mask here is 0x7. If
>> the page pool's base address is 0xffffc021f000, this gives us
>> aligned_start = 0x7. This start is clearly not aligned at a 15 bits
>> boundary (3 bits after shifting by PAGE_SHIFT). It is exactly the
>> opposite. It will never be aligned except when the page pool start also
>> happens to be aligned at that boundary.
>>
>> In the above example, the address of the pointer returned was
>> 0xffffc026e000 which is clearly not 15-bit aligned.
>>
>> This change fixes this problem. Add align_mask to pool_start, and then
>> zero out the bottom mask bits. Adding align_mask ensures that
>> aligned_start is always greater than pool_start.
>>
>> Signed-off-by: Pratyush Yadav <p-yad...@ti.com>
>> ---
>> v2:
>> As suggested by Jan, simplify aligned_start calculation.
>>
>>   hypervisor/paging.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/hypervisor/paging.c b/hypervisor/paging.c
>> index 16687a89..4a24c1f6 100644
>> --- a/hypervisor/paging.c
>> +++ b/hypervisor/paging.c
>> @@ -105,13 +105,15 @@ static unsigned long find_next_free_page(struct 
>> page_pool *pool,
>>   static void *page_alloc_internal(struct page_pool *pool, unsigned int num,
>>                    unsigned long align_mask)
>>   {
>> -    /* The pool itself might not be aligned as required. */
>> -    unsigned long aligned_start =
>> -        ((unsigned long)pool->base_address >> PAGE_SHIFT) & align_mask;
>> -    unsigned long next = aligned_start;
>> -    unsigned long start, last;
>> +    unsigned long aligned_start, pool_start, next, start, last;
>>       unsigned int allocated;
>>   +    pool_start = (unsigned long)pool->base_address >> PAGE_SHIFT;
>> +
>> +    /* The pool itself might not be aligned as required. */
>> +    aligned_start = ((pool_start + align_mask) & ~align_mask) - pool_start;
>> +    next = aligned_start;
>> +
>>   restart:
>>       /* Forward the search start to the next aligned page. */
>>       if ((next - aligned_start) & align_mask)
>>
> 
> Thanks, applied. This should finally close the merge window for the release.

Where do you usually push the patches to? I can't find this one in master or
next on GitHub.

> Jan
> 

-- 
Regards,
Pratyush Yadav

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/dca8f6a9-6f76-f78a-2cdb-3b0203789bfb%40ti.com.
For more options, visit https://groups.google.com/d/optout.