Messages in this thread | | | From | Peng Fan <> | Subject | RE: [PATCH] mm/cma: cma_declare_contiguous: correct err handling | Date | Fri, 15 Feb 2019 01:30:40 +0000 |
| |
Hi Andrew
> -----Original Message----- > From: Andrew Morton [mailto:akpm@linux-foundation.org] > Sent: 2019年2月15日 4:38 > To: Peng Fan <peng.fan@nxp.com> > Cc: labbott@redhat.com; mhocko@suse.com; vbabka@suse.cz; > iamjoonsoo.kim@lge.com; rppt@linux.vnet.ibm.com; > m.szyprowski@samsung.com; rdunlap@infradead.org; > andreyknvl@google.com; linux-mm@kvack.org; linux-kernel@vger.kernel.org; > van.freenix@gmail.com; Mike Rapoport <rppt@linux.ibm.com> > Subject: Re: [PATCH] mm/cma: cma_declare_contiguous: correct err handling > > On Thu, 14 Feb 2019 12:45:51 +0000 Peng Fan <peng.fan@nxp.com> wrote: > > > In case cma_init_reserved_mem failed, need to free the memblock > > allocated by memblock_reserve or memblock_alloc_range. > > > > ... > > > > --- a/mm/cma.c > > +++ b/mm/cma.c > > @@ -353,12 +353,14 @@ int __init cma_declare_contiguous(phys_addr_t > > base, > > > > ret = cma_init_reserved_mem(base, size, order_per_bit, name, > res_cma); > > if (ret) > > - goto err; > > + goto free_mem; > > > > pr_info("Reserved %ld MiB at %pa\n", (unsigned long)size / SZ_1M, > > &base); > > return 0; > > > > +free_mem: > > + memblock_free(base, size); > > err: > > pr_err("Failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M); > > return ret; > > This doesn't look right to me. In the `fixed==true' case we didn't actually > allocate anything and in the `fixed==false' case, the allocated memory is at > `addr', not at `base'.
My code base is 5.0.0-rc6, in mm/cma.c 313 /* Reserve memory */ 314 if (fixed) { 315 if (memblock_is_region_reserved(base, size) || 316 memblock_reserve(base, size) < 0) { 317 ret = -EBUSY; 318 goto err; 319 } 320 } else {
When fixed is true, memblock_is_region_reserved will check whether the [base, base + size) is reserved, if reserved, return -EBUSY, if not reserved, it will call memblock_reserve, if memblock_reserve fail, it will return -EBUSY.
When fixed is false, after memblock_alloc_range, there is one line code `base = addr;`.
Thanks, Peng.
| |