Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12 2014, Joonsoo Kim wrote: > We should free memory for bitmap when we find zone mis-match, > otherwise this memory will leak. > > Additionally, I copy code comment from ppc kvm's cma code to notify > why we need to check zone mis-match. > > Signed-off-by: Joonsoo Kim Reviewed-by: Michal Nazarewicz > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c > index bd0bb81..fb0cdce 100644 > --- a/drivers/base/dma-contiguous.c > +++ b/drivers/base/dma-contiguous.c > @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) > base_pfn = pfn; > for (j = pageblock_nr_pages; j; --j, pfn++) { > WARN_ON_ONCE(!pfn_valid(pfn)); > + /* > + * alloc_contig_range requires the pfn range > + * specified to be in the same zone. Make this > + * simple by forcing the entire CMA resv range > + * to be in the same zone. > + */ > if (page_zone(pfn_to_page(pfn)) != zone) > - return -EINVAL; > + goto err; > } > init_cma_reserved_pageblock(pfn_to_page(base_pfn)); > } while (--i); > > mutex_init(>lock); > return 0; > + > +err: > + kfree(cma->bitmap); > + return -EINVAL; > } -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On 06/12/2014 02:02 PM, Joonsoo Kim wrote: > On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote: >> On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: >>> We should free memory for bitmap when we find zone mis-match, >>> otherwise this memory will leak. >> >> Then, -stable stuff? > > I don't think so. This is just possible leak candidate, so we don't > need to push this to stable tree. > >> >>> >>> Additionally, I copy code comment from ppc kvm's cma code to notify >>> why we need to check zone mis-match. >>> >>> Signed-off-by: Joonsoo Kim >>> >>> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c >>> index bd0bb81..fb0cdce 100644 >>> --- a/drivers/base/dma-contiguous.c >>> +++ b/drivers/base/dma-contiguous.c >>> @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) >>> base_pfn = pfn; >>> for (j = pageblock_nr_pages; j; --j, pfn++) { >>> WARN_ON_ONCE(!pfn_valid(pfn)); >>> + /* >>> +* alloc_contig_range requires the pfn range >>> +* specified to be in the same zone. Make this >>> +* simple by forcing the entire CMA resv range >>> +* to be in the same zone. >>> +*/ >>> if (page_zone(pfn_to_page(pfn)) != zone) >>> - return -EINVAL; >>> + goto err; >> >> At a first glance, I thought it would be better to handle such error >> before activating. >> So when I see the registration code(ie, dma_contiguous_revere_area), >> I realized it is impossible because we didn't set up zone yet. :( >> >> If so, when we detect to fail here, it would be better to report more >> meaningful error message like what was successful zone and what is >> new zone and failed pfn number? > > What I want to do in early phase of this patchset is to make cma code > on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already > has this error handling logic, so I make this patch. > > If we think that we need more things, we can do that on general cma code > after merging this patchset. > Yeah, I also like the idea. After all, this patchset aims to a general CMA management, we could improve more after this patchset. So Acked-by: Zhang Yanfei -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On 06/12/2014 02:02 PM, Joonsoo Kim wrote: On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote: On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Then, -stable stuff? I don't think so. This is just possible leak candidate, so we don't need to push this to stable tree. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* +* alloc_contig_range requires the pfn range +* specified to be in the same zone. Make this +* simple by forcing the entire CMA resv range +* to be in the same zone. +*/ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; At a first glance, I thought it would be better to handle such error before activating. So when I see the registration code(ie, dma_contiguous_revere_area), I realized it is impossible because we didn't set up zone yet. :( If so, when we detect to fail here, it would be better to report more meaningful error message like what was successful zone and what is new zone and failed pfn number? What I want to do in early phase of this patchset is to make cma code on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already has this error handling logic, so I make this patch. If we think that we need more things, we can do that on general cma code after merging this patchset. Yeah, I also like the idea. After all, this patchset aims to a general CMA management, we could improve more after this patchset. So Acked-by: Zhang Yanfei zhangyan...@cn.fujitsu.com -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote: We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com Reviewed-by: Michal Nazarewicz min...@mina86.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* + * alloc_contig_range requires the pfn range + * specified to be in the same zone. Make this + * simple by forcing the entire CMA resv range + * to be in the same zone. + */ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; } init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); mutex_init(cma-lock); return 0; + +err: + kfree(cma-bitmap); + return -EINVAL; } -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote: > On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: > > We should free memory for bitmap when we find zone mis-match, > > otherwise this memory will leak. > > Then, -stable stuff? I don't think so. This is just possible leak candidate, so we don't need to push this to stable tree. > > > > > Additionally, I copy code comment from ppc kvm's cma code to notify > > why we need to check zone mis-match. > > > > Signed-off-by: Joonsoo Kim > > > > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c > > index bd0bb81..fb0cdce 100644 > > --- a/drivers/base/dma-contiguous.c > > +++ b/drivers/base/dma-contiguous.c > > @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) > > base_pfn = pfn; > > for (j = pageblock_nr_pages; j; --j, pfn++) { > > WARN_ON_ONCE(!pfn_valid(pfn)); > > + /* > > +* alloc_contig_range requires the pfn range > > +* specified to be in the same zone. Make this > > +* simple by forcing the entire CMA resv range > > +* to be in the same zone. > > +*/ > > if (page_zone(pfn_to_page(pfn)) != zone) > > - return -EINVAL; > > + goto err; > > At a first glance, I thought it would be better to handle such error > before activating. > So when I see the registration code(ie, dma_contiguous_revere_area), > I realized it is impossible because we didn't set up zone yet. :( > > If so, when we detect to fail here, it would be better to report more > meaningful error message like what was successful zone and what is > new zone and failed pfn number? What I want to do in early phase of this patchset is to make cma code on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already has this error handling logic, so I make this patch. If we think that we need more things, we can do that on general cma code after merging this patchset. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: > We should free memory for bitmap when we find zone mis-match, > otherwise this memory will leak. Then, -stable stuff? > > Additionally, I copy code comment from ppc kvm's cma code to notify > why we need to check zone mis-match. > > Signed-off-by: Joonsoo Kim > > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c > index bd0bb81..fb0cdce 100644 > --- a/drivers/base/dma-contiguous.c > +++ b/drivers/base/dma-contiguous.c > @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) > base_pfn = pfn; > for (j = pageblock_nr_pages; j; --j, pfn++) { > WARN_ON_ONCE(!pfn_valid(pfn)); > + /* > + * alloc_contig_range requires the pfn range > + * specified to be in the same zone. Make this > + * simple by forcing the entire CMA resv range > + * to be in the same zone. > + */ > if (page_zone(pfn_to_page(pfn)) != zone) > - return -EINVAL; > + goto err; At a first glance, I thought it would be better to handle such error before activating. So when I see the registration code(ie, dma_contiguous_revere_area), I realized it is impossible because we didn't set up zone yet. :( If so, when we detect to fail here, it would be better to report more meaningful error message like what was successful zone and what is new zone and failed pfn number? > } > init_cma_reserved_pageblock(pfn_to_page(base_pfn)); > } while (--i); > > mutex_init(>lock); > return 0; > + > +err: > + kfree(cma->bitmap); > + return -EINVAL; > } > > static struct cma cma_areas[MAX_CMA_AREAS]; > -- > 1.7.9.5 -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
Joonsoo Kim writes: > We should free memory for bitmap when we find zone mis-match, > otherwise this memory will leak. > > Additionally, I copy code comment from ppc kvm's cma code to notify > why we need to check zone mis-match. > > Signed-off-by: Joonsoo Kim Reviewed-by: Aneesh Kumar K.V > > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c > index bd0bb81..fb0cdce 100644 > --- a/drivers/base/dma-contiguous.c > +++ b/drivers/base/dma-contiguous.c > @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) > base_pfn = pfn; > for (j = pageblock_nr_pages; j; --j, pfn++) { > WARN_ON_ONCE(!pfn_valid(pfn)); > + /* > + * alloc_contig_range requires the pfn range > + * specified to be in the same zone. Make this > + * simple by forcing the entire CMA resv range > + * to be in the same zone. > + */ > if (page_zone(pfn_to_page(pfn)) != zone) > - return -EINVAL; > + goto err; > } > init_cma_reserved_pageblock(pfn_to_page(base_pfn)); > } while (--i); > > mutex_init(>lock); > return 0; > + > +err: > + kfree(cma->bitmap); > + return -EINVAL; > } > > static struct cma cma_areas[MAX_CMA_AREAS]; > -- > 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 02/10] DMA, CMA: fix possible memory leak
We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* +* alloc_contig_range requires the pfn range +* specified to be in the same zone. Make this +* simple by forcing the entire CMA resv range +* to be in the same zone. +*/ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; } init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); mutex_init(>lock); return 0; + +err: + kfree(cma->bitmap); + return -EINVAL; } static struct cma cma_areas[MAX_CMA_AREAS]; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 02/10] DMA, CMA: fix possible memory leak
We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* +* alloc_contig_range requires the pfn range +* specified to be in the same zone. Make this +* simple by forcing the entire CMA resv range +* to be in the same zone. +*/ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; } init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); mutex_init(cma-lock); return 0; + +err: + kfree(cma-bitmap); + return -EINVAL; } static struct cma cma_areas[MAX_CMA_AREAS]; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
Joonsoo Kim iamjoonsoo@lge.com writes: We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com Reviewed-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* + * alloc_contig_range requires the pfn range + * specified to be in the same zone. Make this + * simple by forcing the entire CMA resv range + * to be in the same zone. + */ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; } init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); mutex_init(cma-lock); return 0; + +err: + kfree(cma-bitmap); + return -EINVAL; } static struct cma cma_areas[MAX_CMA_AREAS]; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Then, -stable stuff? Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* + * alloc_contig_range requires the pfn range + * specified to be in the same zone. Make this + * simple by forcing the entire CMA resv range + * to be in the same zone. + */ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; At a first glance, I thought it would be better to handle such error before activating. So when I see the registration code(ie, dma_contiguous_revere_area), I realized it is impossible because we didn't set up zone yet. :( If so, when we detect to fail here, it would be better to report more meaningful error message like what was successful zone and what is new zone and failed pfn number? } init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); mutex_init(cma-lock); return 0; + +err: + kfree(cma-bitmap); + return -EINVAL; } static struct cma cma_areas[MAX_CMA_AREAS]; -- 1.7.9.5 -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote: On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote: We should free memory for bitmap when we find zone mis-match, otherwise this memory will leak. Then, -stable stuff? I don't think so. This is just possible leak candidate, so we don't need to push this to stable tree. Additionally, I copy code comment from ppc kvm's cma code to notify why we need to check zone mis-match. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index bd0bb81..fb0cdce 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma) base_pfn = pfn; for (j = pageblock_nr_pages; j; --j, pfn++) { WARN_ON_ONCE(!pfn_valid(pfn)); + /* +* alloc_contig_range requires the pfn range +* specified to be in the same zone. Make this +* simple by forcing the entire CMA resv range +* to be in the same zone. +*/ if (page_zone(pfn_to_page(pfn)) != zone) - return -EINVAL; + goto err; At a first glance, I thought it would be better to handle such error before activating. So when I see the registration code(ie, dma_contiguous_revere_area), I realized it is impossible because we didn't set up zone yet. :( If so, when we detect to fail here, it would be better to report more meaningful error message like what was successful zone and what is new zone and failed pfn number? What I want to do in early phase of this patchset is to make cma code on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already has this error handling logic, so I make this patch. If we think that we need more things, we can do that on general cma code after merging this patchset. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/