| Message ID | 20190325081309.6004-1-zbestahu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/cma: Fix crash on CMA allocation if bitmap allocation fails | expand |
On 03/25/2019 01:43 PM, Yue Hu wrote: > From: Yue Hu <huyue2@yulong.com> > > A previous commit f022d8cb7ec7 ("mm: cma: Don't crash on allocation > if CMA area can't be activated") fixes the crash issue when activation > fails via setting cma->count as 0, same logic exists if bitmap > allocation fails. > > Signed-off-by: Yue Hu <huyue2@yulong.com> Looks good to me. Just wondering if cma->count = 0 should be wrapped around in a helper which explicitly states that this cma cannot be used for allocation. Nonetheless. Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
On Mon, 25 Mar 2019 16:13:09 +0800 Yue Hu <zbestahu@gmail.com> wrote: > From: Yue Hu <huyue2@yulong.com> > > A previous commit f022d8cb7ec7 ("mm: cma: Don't crash on allocation > if CMA area can't be activated") fixes the crash issue when activation > fails via setting cma->count as 0, same logic exists if bitmap > allocation fails. > > --- a/mm/cma.c > +++ b/mm/cma.c > @@ -106,8 +106,10 @@ static int __init cma_activate_area(struct cma *cma) > > cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL); > > - if (!cma->bitmap) > + if (!cma->bitmap) { > + cma->count = 0; > return -ENOMEM; > + } > > WARN_ON_ONCE(!pfn_valid(pfn)); > zone = page_zone(pfn_to_page(pfn)); I'm unsure whether this is needed. kmalloc() within __init code is generally considered to be a "can't fail". If this was the only issue then I guess I'd take the patch if only for documentation/clarity purposes. But cma_areas[] is in bss and is guaranteed to be all-zeroes, so I suspect this bug is a can't-happen. And we could revert f022d8cb7ec7 if we could be bothered (I can't).
On Mon, 25 Mar 2019 15:15:41 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Mon, 25 Mar 2019 16:13:09 +0800 Yue Hu <zbestahu@gmail.com> wrote: > > > From: Yue Hu <huyue2@yulong.com> > > > > A previous commit f022d8cb7ec7 ("mm: cma: Don't crash on allocation > > if CMA area can't be activated") fixes the crash issue when activation > > fails via setting cma->count as 0, same logic exists if bitmap > > allocation fails. > > > > --- a/mm/cma.c > > +++ b/mm/cma.c > > @@ -106,8 +106,10 @@ static int __init cma_activate_area(struct cma *cma) > > > > cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL); > > > > - if (!cma->bitmap) > > + if (!cma->bitmap) { > > + cma->count = 0; > > return -ENOMEM; > > + } > > > > WARN_ON_ONCE(!pfn_valid(pfn)); > > zone = page_zone(pfn_to_page(pfn)); > > I'm unsure whether this is needed. > > kmalloc() within __init code is generally considered to be a "can't > fail". > > If this was the only issue then I guess I'd take the patch if only for > documentation/clarity purposes. But cma_areas[] is in bss and is > guaranteed to be all-zeroes, so I suspect this bug is a can't-happen. However, firstly cma->count will be assigned to size >> PAGE_SHIFT in cma_init_reserved_mem(). > And we could revert f022d8cb7ec7 if we could be bothered (I can't). > >
diff --git a/mm/cma.c b/mm/cma.c index f5bf819..991a6ce 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -106,8 +106,10 @@ static int __init cma_activate_area(struct cma *cma) cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL); - if (!cma->bitmap) + if (!cma->bitmap) { + cma->count = 0; return -ENOMEM; + } WARN_ON_ONCE(!pfn_valid(pfn)); zone = page_zone(pfn_to_page(pfn));