Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation

From: Gregory Fong
Date: Fri Feb 27 2015 - 21:07:32 EST


On Fri, Feb 27, 2015 at 5:18 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 27 Feb 2015 17:07:28 -0800 Danesh Petigara <dpetigara@xxxxxxxxxxxx> wrote:
>
>> On 2/27/2015 3:54 PM, Andrew Morton wrote:
>> > On Fri, 27 Feb 2015 15:52:56 -0800 Danesh Petigara <dpetigara@xxxxxxxxxxxx> wrote:
>> >
>> >> On 2/27/2015 1:24 PM, Andrew Morton wrote:
>> >>> On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara <dpetigara@xxxxxxxxxxxx> wrote:
>> >>>
>> >>>> The CMA aligned offset calculation is incorrect for
>> >>>> non-zero order_per_bit values.
>> >>>>
>> >>>> For example, if cma->order_per_bit=1, cma->base_pfn=
>> >>>> 0x2f800000 and align_order=12, the function returns
>> >>>> a value of 0x17c00 instead of 0x400.
>> >>>>
>> >>>> This patch fixes the CMA aligned offset calculation.
>> >>>
>> >>> When fixing a bug please always describe the end-user visible effects
>> >>> of that bug.
>> >>>
>> >>> Without that information others are unable to understand why you are
>> >>> recommending a -stable backport.
>> >>>
>> >>
>> >> Thank you for the feedback. I had no crash logs to show, nevertheless, I
>> >> agree that a sentence describing potential effects of the bug would've
>> >> helped.
>> >
>> > What was the reason for adding a cc:stable?
>> >
>>
>> It was added since the commit that introduced the incorrect logic
>> (b5be83e) was already picked up by v3.19.
>
> argh.
>
> afaict the bug will, under some conditions cause cma_alloc() to report
> that no suitable free area is available in the arena when in fact such
> regions *are* available. So it's effectively a bogus ENOMEM.
>
> Correct? If so, what are the conditions under which this will occur?

This is correct, and it can occur for any nonzero order_per_bit value.
The previous calculation was wrong and would return too-large values
for the offset, so that when cma_alloc looks for free pages in the
bitmap with the requested alignment > order_per_bit, it starts too far
into the bitmap and so CMA allocations will fail despite there
actually being plenty of free pages remaining. It will also probably
have the wrong alignment. With this change, we will get the correct
offset into the bitmap.

One affected user is powerpc KVM, which has kvm_cma->order_per_bit set
to KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, or 18 - 12 = 6.

I actually had written the offset function this way originally, then
tried to make it more like cma_bitmap_aligned_mask(), but screwed up
the transformation and it really wasn't any easier to understand
anyway. That was stupid, sorry about that. =(

Best regards,
Gregory
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/