Re: regression in ath10k dma allocation
On 20.08.19 09:12, Christoph Hellwig wrote:
On Tue, Aug 20, 2019 at 02:58:33PM +0800, Hillf Danton wrote:
On Tue, 20 Aug 2019 05:05:14 +0200 Christoph Hellwig wrote:
Tobias, plase try this patch:
New version below:
---
From b8a805e93be5a5662323b8ac61fe686df839c4ac Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Tue, 20 Aug 2019 11:45:49 +0900
Subject: dma-direct: fix zone selection after an unaddressable CMA allocation
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
Signed-off-by: Christoph Hellwig
---
drivers/iommu/dma-iommu.c | 3 +++
include/linux/dma-contiguous.h | 5 +
kernel/dma/contiguous.c| 9 +++--
kernel/dma/direct.c| 10 +-
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d991d40f797f..f68a62c3c32b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -965,10 +965,13 @@ static void *iommu_dma_alloc_pages(struct device *dev,
size_t size,
{
bool coherent = dev_is_dma_coherent(dev);
size_t alloc_size = PAGE_ALIGN(size);
+ int node = dev_to_node(dev);
struct page *page = NULL;
void *cpu_addr;
page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (!page)
+ page = alloc_pages_node(node, gfp, get_order(alloc_size));
if (!page)
return NULL;
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489..03f8e98e3bcc 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 2bd410f934b3..e6b450fdbeb6 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;
}
@@ -258,6 +254,7 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
* dma_free_contiguous() - release allocated pages
* @dev: Pointer to device for which the pages were allocated.
* @page: Pointer to the allocated pages.
+ int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
* @size: Size of allocated pages.
*
* This function releases memory allocated by dma_alloc_contiguous(). As the
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 795c9b095d75..706113c6bebc 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -85,6 +85,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;
@@ -95,8 +97,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,
_mask);
+ page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (page && !dma_coherent_ok(dev,
Re: regression in ath10k dma allocation
On Tue, Aug 20, 2019 at 02:58:33PM +0800, Hillf Danton wrote:
>
> On Tue, 20 Aug 2019 05:05:14 +0200 Christoph Hellwig wrote:
> >
> > Tobias, plase try this patch:
> >
New version below:
---
>From b8a805e93be5a5662323b8ac61fe686df839c4ac Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Tue, 20 Aug 2019 11:45:49 +0900
Subject: dma-direct: fix zone selection after an unaddressable CMA allocation
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
Signed-off-by: Christoph Hellwig
---
drivers/iommu/dma-iommu.c | 3 +++
include/linux/dma-contiguous.h | 5 +
kernel/dma/contiguous.c| 9 +++--
kernel/dma/direct.c| 10 +-
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d991d40f797f..f68a62c3c32b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -965,10 +965,13 @@ static void *iommu_dma_alloc_pages(struct device *dev,
size_t size,
{
bool coherent = dev_is_dma_coherent(dev);
size_t alloc_size = PAGE_ALIGN(size);
+ int node = dev_to_node(dev);
struct page *page = NULL;
void *cpu_addr;
page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (!page)
+ page = alloc_pages_node(node, gfp, get_order(alloc_size));
if (!page)
return NULL;
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489..03f8e98e3bcc 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 2bd410f934b3..e6b450fdbeb6 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;
}
@@ -258,6 +254,7 @@ struct page *dma_alloc_contiguous(struct device *dev,
size_t size, gfp_t gfp)
* dma_free_contiguous() - release allocated pages
* @dev: Pointer to device for which the pages were allocated.
* @page: Pointer to the allocated pages.
+ int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
* @size: Size of allocated pages.
*
* This function releases memory allocated by dma_alloc_contiguous(). As the
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 795c9b095d75..706113c6bebc 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -85,6 +85,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;
@@ -95,8 +97,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,
_mask);
+ page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+
Re: regression in ath10k dma allocation
On Tue, 20 Aug 2019 05:05:14 +0200 Christoph Hellwig wrote:
>
> Tobias, plase try this patch:
>
A minute!
> --
> >From 88c590a2ecafc8279388f25bfbe1ead8ea3507a6 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig
> Date: Tue, 20 Aug 2019 11:45:49 +0900
> Subject: dma-direct: fix zone selection after an unaddressable CMA allocation
>
> 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
> Signed-off-by: Christoph Hellwig
> ---
> drivers/iommu/dma-iommu.c | 3 +++
> include/linux/dma-contiguous.h | 5 +
> kernel/dma/contiguous.c| 9 +++--
> kernel/dma/direct.c| 7 ++-
> 4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index d991d40f797f..f68a62c3c32b 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -965,10 +965,13 @@ static void *iommu_dma_alloc_pages(struct device *dev,
> size_t size,
> {
> bool coherent = dev_is_dma_coherent(dev);
> size_t alloc_size = PAGE_ALIGN(size);
> + int node = dev_to_node(dev);
> struct page *page = NULL;
> void *cpu_addr;
>
> page = dma_alloc_contiguous(dev, alloc_size, gfp);
> + if (!page)
> + page = alloc_pages_node(node, gfp, get_order(alloc_size));
> if (!page)
> return NULL;
>
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index c05d4e661489..03f8e98e3bcc 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 2bd410f934b3..e6b450fdbeb6 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;
> }
>
> @@ -258,6 +254,7 @@ struct page *dma_alloc_contiguous(struct device *dev,
> size_t size, gfp_t gfp)
> * dma_free_contiguous() - release allocated pages
> * @dev: Pointer to device for which the pages were allocated.
> * @page: Pointer to the allocated pages.
> + int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
> * @size: Size of allocated pages.
> *
> * This function releases memory allocated by dma_alloc_contiguous(). As the
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 795c9b095d75..d82d184463ce 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -85,6 +85,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;
>
> @@ -95,8 +97,11 @@ 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,
> _mask);
> + page = dma_alloc_contiguous(dev, alloc_size, gfp);
> +
Re: regression in ath10k dma allocation
Tobias, plase try this patch:
--
>From 88c590a2ecafc8279388f25bfbe1ead8ea3507a6 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Tue, 20 Aug 2019 11:45:49 +0900
Subject: dma-direct: fix zone selection after an unaddressable CMA allocation
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
Signed-off-by: Christoph Hellwig
---
drivers/iommu/dma-iommu.c | 3 +++
include/linux/dma-contiguous.h | 5 +
kernel/dma/contiguous.c| 9 +++--
kernel/dma/direct.c| 7 ++-
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d991d40f797f..f68a62c3c32b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -965,10 +965,13 @@ static void *iommu_dma_alloc_pages(struct device *dev,
size_t size,
{
bool coherent = dev_is_dma_coherent(dev);
size_t alloc_size = PAGE_ALIGN(size);
+ int node = dev_to_node(dev);
struct page *page = NULL;
void *cpu_addr;
page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (!page)
+ page = alloc_pages_node(node, gfp, get_order(alloc_size));
if (!page)
return NULL;
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index c05d4e661489..03f8e98e3bcc 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 2bd410f934b3..e6b450fdbeb6 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;
}
@@ -258,6 +254,7 @@ struct page *dma_alloc_contiguous(struct device *dev,
size_t size, gfp_t gfp)
* dma_free_contiguous() - release allocated pages
* @dev: Pointer to device for which the pages were allocated.
* @page: Pointer to the allocated pages.
+ int node = dev ? dev_to_node(dev) : NUMA_NO_NODE;
* @size: Size of allocated pages.
*
* This function releases memory allocated by dma_alloc_contiguous(). As the
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 795c9b095d75..d82d184463ce 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -85,6 +85,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;
@@ -95,8 +97,11 @@ 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,
_mask);
+ page = dma_alloc_contiguous(dev, alloc_size, gfp);
+ if (page && dma_coherent_ok(dev, page_to_phys(page), size))
+ return page;
again:
- page = dma_alloc_contiguous(dev, size, gfp);
+ page = alloc_pages_node(node, gfp, get_order(alloc_size));
if (page &&
Re: regression in ath10k dma allocation
On Tue, 20 Aug 2019 10:15:01 +0800 Christoph Hellwig wrote: > On Mon, Aug 19, 2019 at 06:58:52PM -0700, Nicolin Chen wrote: > > Right...the condition was in-between. However, not every caller > > of dma_alloc_contiguous() is supposed to have a coherent check. > > So we either add a 'bool coherent_ok' to the API or revert the > > dma-direct part back to the original. Probably former option is > > better? > > > > Thank you for the debugging. I have been a bit distracted, may > > not be able to submit a fix very soon. Would you like to help? > > Yeah, it turns out that while the idea for the dma_alloc_contiguous > helper was neat it didn't work out at all, and me pushing Nicolin > down that route was not a very smart idea. Sorry for causing this > mess. > > I think we'll just need to open code it for dma-direct for 5.3. > Hillf do you want to cook up a patch or should I do it? Feel free to do that, Sir. Thanks Hillf ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: regression in ath10k dma allocation
On Mon, Aug 19, 2019 at 06:58:52PM -0700, Nicolin Chen wrote: > Right...the condition was in-between. However, not every caller > of dma_alloc_contiguous() is supposed to have a coherent check. > So we either add a 'bool coherent_ok' to the API or revert the > dma-direct part back to the original. Probably former option is > better? > > Thank you for the debugging. I have been a bit distracted, may > not be able to submit a fix very soon. Would you like to help? Yeah, it turns out that while the idea for the dma_alloc_contiguous helper was neat it didn't work out at all, and me pushing Nicolin down that route was not a very smart idea. Sorry for causing this mess. I think we'll just need to open code it for dma-direct for 5.3. Hillf do you want to cook up a patch or should I do it? ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: regression in ath10k dma allocation
Hello Hillf,
On Mon, Aug 19, 2019 at 12:38:38AM +0200, Tobias Klausmann wrote:
>
> On 18.08.19 05:13, Hillf Danton wrote:
> > On Sat, 17 Aug 2019 00:42:48 +0200 Tobias Klausmann wrote:
> > > Hi Nicolin,
> > >
> > > On 17.08.19 00:25, Nicolin Chen wrote:
> > > > Hi Tobias
> > > >
> > > > On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
> > > > > > do you have CONFIG_DMA_CMA set in your config? If not please make
> > > > > > sure
> > > > > > you have this commit in your testing tree, and if the problem still
> > > > > > persists it would be a little odd and we'd have to dig deeper:
> > > > > >
> > > > > > commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
> > > > > > Author: Nicolin Chen
> > > > > > Date: Wed May 29 17:54:25 2019 -0700
> > > > > >
> > > > > >dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
> > > > > > free}_contiguous()
> > > > > yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you
> > > > > mention
> > > > > above is included, if you have any hints how to go forward, please
> > > > > let me
> > > > > know!
> > > > For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
> > > > feel this one should work for you. Would you please check if it
> > > > is included or try it out otherwise?
> > > >
> > > > dma-contiguous: do not overwrite align in dma_alloc_contiguous()
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
> > >
> > > Thanks for the hint, yet the commit is included and does not fix the
> > > problem!
> > >
> Hi Hillf,
>
> i just tested you first hunk (which comes from kernel/dma/direct.c if i'm
> not mistaken), it did not compile on its own, yet with a tiny bit of work it
> did, and it does indeed solve the regression. But if using that is the
> "right" way to do it, not sure, but its not on me to decide.
>
> Anyway: Thanks for the hint,
>
> Tobias
>
>
> > Hi Tobias
> >
> > Two minor diffs below in hope that they might make sense.
> >
> > 1, fallback unless dma coherent ok.
> >
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -246,6 +246,10 @@ struct page *dma_alloc_contiguous(struct
> > size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
> > page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
> > + if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
> > + dma_free_contiguous(dev, page, size);
> > + page = NULL;
> > + }
Right...the condition was in-between. However, not every caller
of dma_alloc_contiguous() is supposed to have a coherent check.
So we either add a 'bool coherent_ok' to the API or revert the
dma-direct part back to the original. Probably former option is
better?
Thank you for the debugging. I have been a bit distracted, may
not be able to submit a fix very soon. Would you like to help?
Thanks!
Nicolin
> > }
> > /* Fallback allocation of normal pages */
> > --
> >
> > 2, cleanup: cma unless contiguous
> >
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -234,18 +234,13 @@ struct page *dma_alloc_contiguous(struct
> > size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > size_t align = get_order(PAGE_ALIGN(size));
> > struct page *page = NULL;
> > - struct cma *cma = NULL;
> > -
> > - if (dev && dev->cma_area)
> > - cma = dev->cma_area;
> > - else if (count > 1)
> > - cma = dma_contiguous_default_area;
> > /* CMA can be used only in the context which permits sleeping */
> > - if (cma && gfpflags_allow_blocking(gfp)) {
> > + if (count > 1 && gfpflags_allow_blocking(gfp)) {
> > size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
> > - page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
> > + page = cma_alloc(dev_get_cma_area(dev), count, cma_align,
> > + gfp & __GFP_NOWARN);
> > if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
> > dma_free_contiguous(dev, page, size);
> > page = NULL;
> > --
> >
Re: regression in ath10k dma allocation
On 18.08.19 05:13, Hillf Danton wrote:
On Sat, 17 Aug 2019 00:42:48 +0200 Tobias Klausmann wrote:
Hi Nicolin,
On 17.08.19 00:25, Nicolin Chen wrote:
Hi Tobias
On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
do you have CONFIG_DMA_CMA set in your config? If not please make sure
you have this commit in your testing tree, and if the problem still
persists it would be a little odd and we'd have to dig deeper:
commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
Author: Nicolin Chen
Date: Wed May 29 17:54:25 2019 -0700
dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
free}_contiguous()
yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
above is included, if you have any hints how to go forward, please let me
know!
For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
feel this one should work for you. Would you please check if it
is included or try it out otherwise?
dma-contiguous: do not overwrite align in dma_alloc_contiguous()
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
Thanks for the hint, yet the commit is included and does not fix the
problem!
Hi Hillf,
i just tested you first hunk (which comes from kernel/dma/direct.c if
i'm not mistaken), it did not compile on its own, yet with a tiny bit of
work it did, and it does indeed solve the regression. But if using that
is the "right" way to do it, not sure, but its not on me to decide.
Anyway: Thanks for the hint,
Tobias
Hi Tobias
Two minor diffs below in hope that they might make sense.
1, fallback unless dma coherent ok.
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -246,6 +246,10 @@ struct page *dma_alloc_contiguous(struct
size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
+ if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+ dma_free_contiguous(dev, page, size);
+ page = NULL;
+ }
}
/* Fallback allocation of normal pages */
--
2, cleanup: cma unless contiguous
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -234,18 +234,13 @@ struct page *dma_alloc_contiguous(struct
size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
size_t align = get_order(PAGE_ALIGN(size));
struct page *page = NULL;
- struct cma *cma = NULL;
-
- if (dev && dev->cma_area)
- cma = dev->cma_area;
- else if (count > 1)
- cma = dma_contiguous_default_area;
/* CMA can be used only in the context which permits sleeping */
- if (cma && gfpflags_allow_blocking(gfp)) {
+ if (count > 1 && gfpflags_allow_blocking(gfp)) {
size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
- page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
+ page = cma_alloc(dev_get_cma_area(dev), count, cma_align,
+ gfp & __GFP_NOWARN);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
dma_free_contiguous(dev, page, size);
page = NULL;
--
Re: regression in ath10k dma allocation
On Sat, 17 Aug 2019 00:42:48 +0200 Tobias Klausmann wrote:
>Hi Nicolin,
>
>On 17.08.19 00:25, Nicolin Chen wrote:
>> Hi Tobias
>>
>> On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
do you have CONFIG_DMA_CMA set in your config? If not please make sure
you have this commit in your testing tree, and if the problem still
persists it would be a little odd and we'd have to dig deeper:
commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
Author: Nicolin Chen
Date: Wed May 29 17:54:25 2019 -0700
dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
free}_contiguous()
>>> yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
>>> above is included, if you have any hints how to go forward, please let me
>>> know!
>> For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
>> feel this one should work for you. Would you please check if it
>> is included or try it out otherwise?
>>
>> dma-contiguous: do not overwrite align in dma_alloc_contiguous()
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
>
>
>Thanks for the hint, yet the commit is included and does not fix the
>problem!
>
Hi Tobias
Two minor diffs below in hope that they might make sense.
1, fallback unless dma coherent ok.
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -246,6 +246,10 @@ struct page *dma_alloc_contiguous(struct
size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
+ if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+ dma_free_contiguous(dev, page, size);
+ page = NULL;
+ }
}
/* Fallback allocation of normal pages */
--
2, cleanup: cma unless contiguous
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -234,18 +234,13 @@ struct page *dma_alloc_contiguous(struct
size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT;
size_t align = get_order(PAGE_ALIGN(size));
struct page *page = NULL;
- struct cma *cma = NULL;
-
- if (dev && dev->cma_area)
- cma = dev->cma_area;
- else if (count > 1)
- cma = dma_contiguous_default_area;
/* CMA can be used only in the context which permits sleeping */
- if (cma && gfpflags_allow_blocking(gfp)) {
+ if (count > 1 && gfpflags_allow_blocking(gfp)) {
size_t cma_align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT);
- page = cma_alloc(cma, count, cma_align, gfp & __GFP_NOWARN);
+ page = cma_alloc(dev_get_cma_area(dev), count, cma_align,
+ gfp & __GFP_NOWARN);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
dma_free_contiguous(dev, page, size);
page = NULL;
--
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
Re: regression in ath10k dma allocation
Hi Nicolin,
On 17.08.19 00:25, Nicolin Chen wrote:
Hi Tobias
On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
do you have CONFIG_DMA_CMA set in your config? If not please make sure
you have this commit in your testing tree, and if the problem still
persists it would be a little odd and we'd have to dig deeper:
commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
Author: Nicolin Chen
Date: Wed May 29 17:54:25 2019 -0700
dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
free}_contiguous()
yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
above is included, if you have any hints how to go forward, please let me
know!
For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
feel this one should work for you. Would you please check if it
is included or try it out otherwise?
dma-contiguous: do not overwrite align in dma_alloc_contiguous()
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
Thanks for the hint, yet the commit is included and does not fix the
problem!
Greetings,
Tobias
Re: regression in ath10k dma allocation
Hi Tobias
On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
> > do you have CONFIG_DMA_CMA set in your config? If not please make sure
> > you have this commit in your testing tree, and if the problem still
> > persists it would be a little odd and we'd have to dig deeper:
> >
> > commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
> > Author: Nicolin Chen
> > Date: Wed May 29 17:54:25 2019 -0700
> >
> > dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
> > free}_contiguous()
> yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
> above is included, if you have any hints how to go forward, please let me
> know!
For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
feel this one should work for you. Would you please check if it
is included or try it out otherwise?
dma-contiguous: do not overwrite align in dma_alloc_contiguous()
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
Thanks
Nicolin
Re: regression in ath10k dma allocation
Hi Tobias,
do you have CONFIG_DMA_CMA set in your config? If not please make sure
you have this commit in your testing tree, and if the problem still
persists it would be a little odd and we'd have to dig deeper:
commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
Author: Nicolin Chen
Date: Wed May 29 17:54:25 2019 -0700
dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc,
free}_contiguous()
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
regression in ath10k dma allocation
Hello all,
within the current development cycle i noted the ath10k driver failing
to setup:
[ 3.185660] ath10k_pci :02:00.0: failed to alloc CE dest ring 1: -12
[ 3.185664] ath10k_pci :02:00.0: failed to allocate copy engine
pipe 1: -12
[ 3.185667] ath10k_pci :02:00.0: failed to allocate copy engine
pipes: -12
[ 3.185669] ath10k_pci :02:00.0: failed to setup resource: -12
[ 3.185692] ath10k_pci: probe of :02:00.0 failed with error -12
the actual failure comes from [1] and indeed bisecting brought me to a
related commit "dma-contiguous: add dma_{alloc,free}_contiguous()
helpers" [2]. Reverting the commit fixes the problem, yet this might
just be the driver abusing the dma infrastructure, so hopefully someone
can have a look at it, as i'm not familiar with the code!
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ath/ath10k/ce.c?h=v5.3-rc4#n1650
[2]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b1d2dc009dece4cd7e629419b52266ba51960a6b
Greetings,
Tobias
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
