[PATCH 0/1][Eoan][SRU Disco] Fix additional regression in CMA/DMA rework

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][Eoan][SRU Disco] Fix additional regression in CMA/DMA rework

dann frazier-4
BugLink: https://bugs.launchpad.net/bugs/1841483

As noted in the patch, my backport drops the upstream hunk that applied to
dma-iommu.c, as it wasn't yet converted over to the new interface until
after v5.2. I wouldn't mind a 2nd set of eyes confirming this approach.
Regression tested on a HiSilicon D06.

Christoph Hellwig (1):
  dma-direct: fix zone selection after an unaddressable CMA allocation

 include/linux/dma-contiguous.h |  5 +----
 kernel/dma/contiguous.c        |  8 ++------
 kernel/dma/direct.c            | 10 +++++++++-
 3 files changed, 12 insertions(+), 11 deletions(-)

--
2.23.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][Eoan][SRU Disco] dma-direct: fix zone selection after an unaddressable CMA allocation

dann frazier-4
From: Christoph Hellwig <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1841483

The new dma_alloc_contiguous hides if we allocate CMA or regular
pages, and thus fails to retry a ZONE_NORMAL allocation if the CMA
allocation succeeds but isn't addressable.  That means we either fail
outright or dip into a small zone that might not succeed either.

Thanks to Hillf Danton for debugging this issue.

Fixes: b1d2dc009dec ("dma-contiguous: add dma_{alloc,free}_contiguous() helpers")
Reported-by: Tobias Klausmann <[hidden email]>
Signed-off-by: Christoph Hellwig <[hidden email]>
Tested-by: Tobias Klausmann <[hidden email]>
(backported from commit 90ae409f9eb3bcaf38688f9ec22375816053a08e)
[ dannf: dropped dma-iommu.c changes, as that didn't switch over to the
  new dma_alloc_contiguous() interface until after v5.2 ]
Signed-off-by: dann frazier <[hidden email]>
---
 include/linux/dma-contiguous.h |  5 +----
 kernel/dma/contiguous.c        |  8 ++------
 kernel/dma/direct.c            | 10 +++++++++-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489b..03f8e98e3bcc9 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -160,10 +160,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
 static inline struct page *dma_alloc_contiguous(struct device *dev, size_t size,
  gfp_t gfp)
 {
- int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
- size_t align = get_order(PAGE_ALIGN(size));
-
- return alloc_pages_node(node, gfp, align);
+ return NULL;
 }
 
 static inline void dma_free_contiguous(struct device *dev, struct page *page,
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 2bd410f934b32..69cfb4345388c 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -230,9 +230,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
  */
 struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
 {
- int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
- size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
- size_t align = get_order(PAGE_ALIGN(size));
+ size_t count = size >> PAGE_SHIFT;
  struct page *page = NULL;
  struct cma *cma = NULL;
 
@@ -243,14 +241,12 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
 
  /* CMA can be used only in the context which permits sleeping */
  if (cma && gfpflags_allow_blocking(gfp)) {
+ size_t align = get_order(size);
  size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
 
  page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
  }
 
- /* Fallback allocation of normal pages */
- if (!page)
- page = alloc_pages_node(node, gfp, align);
  return page;
 }
 
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 0816c1e8b05af..18596b804d384 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -96,6 +96,8 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
  dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
 {
+ size_t alloc_size = PAGE_ALIGN(size);
+ int node = dev_to_node(dev);
  struct page *page = NULL;
  u64 phys_mask;
 
@@ -106,8 +108,14 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
  gfp &= ~__GFP_ZERO;
  gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
  &phys_mask);
+ page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+ dma_free_contiguous(dev, page, alloc_size);
+ page = NULL;
+ }
 again:
- page = dma_alloc_contiguous(dev, size, gfp);
+ if (!page)
+ page = alloc_pages_node(node, gfp, get_order(alloc_size));
  if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
  dma_free_contiguous(dev, page, size);
  page = NULL;
--
2.23.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK / APPLIED[E]: [PATCH 0/1][Eoan][SRU Disco] Fix additional regression in CMA/DMA rework

Seth Forshee
In reply to this post by dann frazier-4
On Mon, Aug 26, 2019 at 07:26:07PM -0600, dann frazier wrote:
> BugLink: https://bugs.launchpad.net/bugs/1841483
>
> As noted in the patch, my backport drops the upstream hunk that applied to
> dma-iommu.c, as it wasn't yet converted over to the new interface until
> after v5.2. I wouldn't mind a 2nd set of eyes confirming this approach.
> Regression tested on a HiSilicon D06.

I took a look, I agree that skipping the hunk for dma-iommu.c is
correct. It looks like the relevant code for disco is already using
alloc_pages_node() as the upstream patch adds as a fallback case. So the
backport looks good to me.

Acked-by: Seth Forshee <[hidden email]>

Applied to eoan/master-next, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1][Eoan][SRU Disco] dma-direct: fix zone selection after an unaddressable CMA allocation

Kleber Souza
In reply to this post by dann frazier-4
On 8/27/19 3:26 AM, dann frazier wrote:

> From: Christoph Hellwig <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1841483
>
> The new dma_alloc_contiguous hides if we allocate CMA or regular
> pages, and thus fails to retry a ZONE_NORMAL allocation if the CMA
> allocation succeeds but isn't addressable.  That means we either fail
> outright or dip into a small zone that might not succeed either.
>
> Thanks to Hillf Danton for debugging this issue.
>
> Fixes: b1d2dc009dec ("dma-contiguous: add dma_{alloc,free}_contiguous() helpers")
> Reported-by: Tobias Klausmann <[hidden email]>
> Signed-off-by: Christoph Hellwig <[hidden email]>
> Tested-by: Tobias Klausmann <[hidden email]>
> (backported from commit 90ae409f9eb3bcaf38688f9ec22375816053a08e)
> [ dannf: dropped dma-iommu.c changes, as that didn't switch over to the
>   new dma_alloc_contiguous() interface until after v5.2 ]
> Signed-off-by: dann frazier <[hidden email]>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  include/linux/dma-contiguous.h |  5 +----
>  kernel/dma/contiguous.c        |  8 ++------
>  kernel/dma/direct.c            | 10 +++++++++-
>  3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index c05d4e661489b..03f8e98e3bcc9 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -160,10 +160,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
>  static inline struct page *dma_alloc_contiguous(struct device *dev, size_t size,
>   gfp_t gfp)
>  {
> - int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> - size_t align = get_order(PAGE_ALIGN(size));
> -
> - return alloc_pages_node(node, gfp, align);
> + return NULL;
>  }
>  
>  static inline void dma_free_contiguous(struct device *dev, struct page *page,
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 2bd410f934b32..69cfb4345388c 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -230,9 +230,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
>   */
>  struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
>  {
> - int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> - size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> - size_t align = get_order(PAGE_ALIGN(size));
> + size_t count = size >> PAGE_SHIFT;
>   struct page *page = NULL;
>   struct cma *cma = NULL;
>  
> @@ -243,14 +241,12 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
>  
>   /* CMA can be used only in the context which permits sleeping */
>   if (cma && gfpflags_allow_blocking(gfp)) {
> + size_t align = get_order(size);
>   size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
>  
>   page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
>   }
>  
> - /* Fallback allocation of normal pages */
> - if (!page)
> - page = alloc_pages_node(node, gfp, align);
>   return page;
>  }
>  
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 0816c1e8b05af..18596b804d384 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -96,6 +96,8 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>  struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>   dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>  {
> + size_t alloc_size = PAGE_ALIGN(size);
> + int node = dev_to_node(dev);
>   struct page *page = NULL;
>   u64 phys_mask;
>  
> @@ -106,8 +108,14 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
>   gfp &= ~__GFP_ZERO;
>   gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
>   &phys_mask);
> + page = dma_alloc_contiguous(dev, alloc_size, gfp);
> + if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
> + dma_free_contiguous(dev, page, alloc_size);
> + page = NULL;
> + }
>  again:
> - page = dma_alloc_contiguous(dev, size, gfp);
> + if (!page)
> + page = alloc_pages_node(node, gfp, get_order(alloc_size));
>   if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
>   dma_free_contiguous(dev, page, size);
>   page = NULL;
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[D]: [PATCH 0/1][Eoan][SRU Disco] Fix additional regression in CMA/DMA rework

Kleber Souza
In reply to this post by dann frazier-4
On 8/27/19 3:26 AM, dann frazier wrote:

> BugLink: https://bugs.launchpad.net/bugs/1841483
>
> As noted in the patch, my backport drops the upstream hunk that applied to
> dma-iommu.c, as it wasn't yet converted over to the new interface until
> after v5.2. I wouldn't mind a 2nd set of eyes confirming this approach.
> Regression tested on a HiSilicon D06.
>
> Christoph Hellwig (1):
>   dma-direct: fix zone selection after an unaddressable CMA allocation
>
>  include/linux/dma-contiguous.h |  5 +----
>  kernel/dma/contiguous.c        |  8 ++------
>  kernel/dma/direct.c            | 10 +++++++++-
>  3 files changed, 12 insertions(+), 11 deletions(-)
>

Applied to disco/master-next branch.

Thanks,
Kleber


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team