From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <20150227171811.c9f6d0ca.akpm@linux-foundation.org> References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> <54F10358.1050102@broadcom.com> <20150227155458.697b7701d0a67ff7b4f3d9cb@linux-foundation.org> <54F114D0.3060306@broadcom.com> <20150227171811.c9f6d0ca.akpm@linux-foundation.org> From: Gregory Fong Date: Fri, 27 Feb 2015 18:06:57 -0800 Message-ID: Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation To: Andrew Morton Cc: Danesh Petigara , Marek Szyprowski , Michal Nazarewicz , Joonsoo Kim , "Aneesh Kumar K.V" , Laurent Pinchart , linux-mm@kvack.org, "linux-kernel@vger.kernel.org" , stable@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: On Fri, Feb 27, 2015 at 5:18 PM, Andrew Morton wrote: > On Fri, 27 Feb 2015 17:07:28 -0800 Danesh Petigara wrote: > >> On 2/27/2015 3:54 PM, Andrew Morton wrote: >> > On Fri, 27 Feb 2015 15:52:56 -0800 Danesh Petigara wrote: >> > >> >> On 2/27/2015 1:24 PM, Andrew Morton wrote: >> >>> On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 27 Feb 2015 17:18:11 -0800 From: Andrew Morton To: Danesh Petigara Cc: , , , , , , , , Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation Message-Id: <20150227171811.c9f6d0ca.akpm@linux-foundation.org> In-Reply-To: <54F114D0.3060306@broadcom.com> References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> <54F10358.1050102@broadcom.com> <20150227155458.697b7701d0a67ff7b4f3d9cb@linux-foundation.org> <54F114D0.3060306@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On Fri, 27 Feb 2015 17:07:28 -0800 Danesh Petigara wrote: > On 2/27/2015 3:54 PM, Andrew Morton wrote: > > On Fri, 27 Feb 2015 15:52:56 -0800 Danesh Petigara wrote: > > > >> On 2/27/2015 1:24 PM, Andrew Morton wrote: > >>> On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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 isn't hard - I want to know what the patch *does*! -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <54F114D0.3060306@broadcom.com> Date: Fri, 27 Feb 2015 17:07:28 -0800 From: Danesh Petigara MIME-Version: 1.0 To: Andrew Morton CC: , , , , , , , , Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> <54F10358.1050102@broadcom.com> <20150227155458.697b7701d0a67ff7b4f3d9cb@linux-foundation.org> In-Reply-To: <20150227155458.697b7701d0a67ff7b4f3d9cb@linux-foundation.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 2/27/2015 3:54 PM, Andrew Morton wrote: > On Fri, 27 Feb 2015 15:52:56 -0800 Danesh Petigara wrote: > >> On 2/27/2015 1:24 PM, Andrew Morton wrote: >>> On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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. Thanks, Danesh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 27 Feb 2015 15:54:58 -0800 From: Andrew Morton To: Danesh Petigara Cc: , , , , , , , , Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation Message-Id: <20150227155458.697b7701d0a67ff7b4f3d9cb@linux-foundation.org> In-Reply-To: <54F10358.1050102@broadcom.com> References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> <54F10358.1050102@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On Fri, 27 Feb 2015 15:52:56 -0800 Danesh Petigara wrote: > On 2/27/2015 1:24 PM, Andrew Morton wrote: > > On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <54F10358.1050102@broadcom.com> Date: Fri, 27 Feb 2015 15:52:56 -0800 From: Danesh Petigara MIME-Version: 1.0 To: Andrew Morton CC: , , , , , , , , Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> In-Reply-To: <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 2/27/2015 1:24 PM, Andrew Morton wrote: > On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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. I'll keep that in mind for future submissions. Best Regards, Danesh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 27 Feb 2015 13:24:43 -0800 From: Andrew Morton To: Danesh Petigara Cc: , , , , , , , , Subject: Re: [PATCH v2] mm: cma: fix CMA aligned offset calculation Message-Id: <20150227132443.e17d574d45451f10f413f065@linux-foundation.org> In-Reply-To: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> References: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On Tue, 24 Feb 2015 15:39:45 -0800 Danesh Petigara 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. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Danesh Petigara To: CC: , , , , , , , , Danesh Petigara , Subject: [PATCH v2] mm: cma: fix CMA aligned offset calculation Date: Tue, 24 Feb 2015 15:39:45 -0800 Message-ID: <1424821185-16956-1-git-send-email-dpetigara@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain Sender: owner-linux-mm@kvack.org List-ID: 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. Cc: stable@vger.kernel.org Signed-off-by: Danesh Petigara Reviewed-by: Gregory Fong Acked-by: Michal Nazarewicz --- Changes since v1: - moved comment out of function - removed unused 'alignment' variable v1: https://lkml.org/lkml/2015/2/24/598 mm/cma.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/mm/cma.c b/mm/cma.c index 75016fd..68ecb7a 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -64,15 +64,17 @@ static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int align_order) return (1UL << (align_order - cma->order_per_bit)) - 1; } +/* + * Find a PFN aligned to the specified order and return an offset represented in + * order_per_bits. + */ static unsigned long cma_bitmap_aligned_offset(struct cma *cma, int align_order) { - unsigned int alignment; - if (align_order <= cma->order_per_bit) return 0; - alignment = 1UL << (align_order - cma->order_per_bit); - return ALIGN(cma->base_pfn, alignment) - - (cma->base_pfn >> cma->order_per_bit); + + return (ALIGN(cma->base_pfn, (1UL << align_order)) + - cma->base_pfn) >> cma->order_per_bit; } static unsigned long cma_bitmap_maxno(struct cma *cma) -- 1.9.1 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org