Message ID | 20211126074904.88388-1-guangming.cao@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v4] dma-buf: system_heap: Use 'for_each_sgtable_sg' in pages free flow | expand |
Am 26.11.21 um 08:49 schrieb guangming.cao@mediatek.com: > From: Guangming <Guangming.Cao@mediatek.com> > > For previous version, it uses 'sg_table.nent's to traverse sg_table in pages > free flow. > However, 'sg_table.nents' is reassigned in 'dma_map_sg', it means the number of > created entries in the DMA adderess space. > So, use 'sg_table.nents' in pages free flow will case some pages can't be freed. > > Here we should use sg_table.orig_nents to free pages memory, but use the > sgtable helper 'for each_sgtable_sg'(, instead of the previous rather common > helper 'for_each_sg' which maybe cause memory leak) is much better. > > Fixes: d963ab0f15fb0 ("dma-buf: system_heap: Allocate higher order pages if available") > Signed-off-by: Guangming <Guangming.Cao@mediatek.com> > Reviewed-by: Robin Murphy <robin.murphy@arm.com> Reviewed-by: Christian König <christian.koenig@amd.com> > Cc: <stable@vger.kernel.org> # 5.11.* > --- > v4: Correct commit message > 1. Cc stable@vger.kernel.org in commit message and add required kernel version. > 2. Add reviewed-by since patch V2 and V4 are same and V2 is reviewed by Robin. > 3. There is no new code change in V4. > V3: Cc stable@vger.kernel.org > 1. This patch needs to be merged stable branch, add stable@vger.kernel.org > in mail list. > 2. Correct some spelling mistake. > 3. There is No new code change in V3. > V2: use 'for_each_sgtable_sg' to 'replece for_each_sg' as suggested by Robin. > > --- > drivers/dma-buf/heaps/system_heap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c > index 23a7e74ef966..8660508f3684 100644 > --- a/drivers/dma-buf/heaps/system_heap.c > +++ b/drivers/dma-buf/heaps/system_heap.c > @@ -289,7 +289,7 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) > int i; > > table = &buffer->sg_table; > - for_each_sg(table->sgl, sg, table->nents, i) { > + for_each_sgtable_sg(table, sg, i) { > struct page *page = sg_page(sg); > > __free_pages(page, compound_order(page));
On Thu, Nov 25, 2021 at 11:48 PM <guangming.cao@mediatek.com> wrote: > > From: Guangming <Guangming.Cao@mediatek.com> > > For previous version, it uses 'sg_table.nent's to traverse sg_table in pages > free flow. > However, 'sg_table.nents' is reassigned in 'dma_map_sg', it means the number of > created entries in the DMA adderess space. > So, use 'sg_table.nents' in pages free flow will case some pages can't be freed. > > Here we should use sg_table.orig_nents to free pages memory, but use the > sgtable helper 'for each_sgtable_sg'(, instead of the previous rather common > helper 'for_each_sg' which maybe cause memory leak) is much better. > > Fixes: d963ab0f15fb0 ("dma-buf: system_heap: Allocate higher order pages if available") > Signed-off-by: Guangming <Guangming.Cao@mediatek.com> > Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Cc: <stable@vger.kernel.org> # 5.11.* Thanks so much for catching this and sending in all the revisions! Reviewed-by: John Stultz <john.stultz@linaro.org>
Hello Guangming, On Mon, 29 Nov 2021 at 23:35, John Stultz <john.stultz@linaro.org> wrote: > > On Thu, Nov 25, 2021 at 11:48 PM <guangming.cao@mediatek.com> wrote: > > > > From: Guangming <Guangming.Cao@mediatek.com> > > > > For previous version, it uses 'sg_table.nent's to traverse sg_table in pages > > free flow. > > However, 'sg_table.nents' is reassigned in 'dma_map_sg', it means the number of > > created entries in the DMA adderess space. > > So, use 'sg_table.nents' in pages free flow will case some pages can't be freed. > > > > Here we should use sg_table.orig_nents to free pages memory, but use the > > sgtable helper 'for each_sgtable_sg'(, instead of the previous rather common > > helper 'for_each_sg' which maybe cause memory leak) is much better. Thanks for catching this and the patch; applied to drm-misc-fixes. > > > > Fixes: d963ab0f15fb0 ("dma-buf: system_heap: Allocate higher order pages if available") > > Signed-off-by: Guangming <Guangming.Cao@mediatek.com> > > Reviewed-by: Robin Murphy <robin.murphy@arm.com> > > Cc: <stable@vger.kernel.org> # 5.11.* > > Thanks so much for catching this and sending in all the revisions! > > Reviewed-by: John Stultz <john.stultz@linaro.org> Best, Sumit.
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 23a7e74ef966..8660508f3684 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -289,7 +289,7 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) int i; table = &buffer->sg_table; - for_each_sg(table->sgl, sg, table->nents, i) { + for_each_sgtable_sg(table, sg, i) { struct page *page = sg_page(sg); __free_pages(page, compound_order(page));