* [PATCH v2] dma-direct: do not allocate a single page from CMA area
@ 2019-01-15 21:51 Nicolin Chen
2019-02-04 8:23 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2019-01-15 21:51 UTC (permalink / raw)
To: hch, m.szyprowski, robin.murphy; +Cc: vdumpa, iommu, linux-kernel
The addresses within a single page are always contiguous, so it's
not so necessary to allocate one single page from CMA area. Since
the CMA area has a limited predefined size of space, it might run
out of space in some heavy use case, where there might be quite a
lot CMA pages being allocated for single pages.
This patch tries to skip CMA allocations of single pages and lets
them go through normal page allocations unless the allocation has
a DMA_ATTR_FORCE_CONTIGUOUS attribute. This'd save some resources
in the CMA area for further more CMA allocations, and it can also
reduce CMA fragmentations resulted from trivial allocations.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
Robin/Christoph,
I have some personal priority to submit this patch. I understand
you might have other plan to clean up the code first. Just would
it be possible for you to review and apply this one if it doesn't
conflict too much? Thanks!
Changelog
v1->v2:
* Added DMA_ATTR_FORCE_CONTIGUOUS flag check so as to enforce
CMA allocations if callers specified.
* Added to the commit message the reduction of fragmentations
suggested by Robin.
kernel/dma/direct.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..5d57f99b2edf 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -109,8 +109,14 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
&phys_mask);
again:
- /* CMA can be used only in the context which permits sleeping */
- if (gfpflags_allow_blocking(gfp)) {
+ /*
+ * CMA can be used only in the context which permits sleeping.
+ * Since addresses within one PAGE are always contiguous, skip
+ * CMA allocation for a single page to save CMA reserved space
+ * unless DMA_ATTR_FORCE_CONTIGUOUS is flagged.
+ */
+ if (gfpflags_allow_blocking(gfp) &&
+ (count > 1 || attrs & DMA_ATTR_FORCE_CONTIGUOUS)) {
page = dma_alloc_from_contiguous(dev, count, page_order,
gfp & __GFP_NOWARN);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area
2019-01-15 21:51 [PATCH v2] dma-direct: do not allocate a single page from CMA area Nicolin Chen
@ 2019-02-04 8:23 ` Christoph Hellwig
2019-02-05 23:05 ` Nicolin Chen
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-02-04 8:23 UTC (permalink / raw)
To: Nicolin Chen; +Cc: hch, m.szyprowski, robin.murphy, vdumpa, iommu, linux-kernel
On Tue, Jan 15, 2019 at 01:51:40PM -0800, Nicolin Chen wrote:
> The addresses within a single page are always contiguous, so it's
> not so necessary to allocate one single page from CMA area. Since
> the CMA area has a limited predefined size of space, it might run
> out of space in some heavy use case, where there might be quite a
> lot CMA pages being allocated for single pages.
>
> This patch tries to skip CMA allocations of single pages and lets
> them go through normal page allocations unless the allocation has
> a DMA_ATTR_FORCE_CONTIGUOUS attribute. This'd save some resources
> in the CMA area for further more CMA allocations, and it can also
> reduce CMA fragmentations resulted from trivial allocations.
That DMA_ATTR_FORCE_CONTIGUOUS flag does not make sense. A single
page allocation is per defintion always contigous.
> again:
> - /* CMA can be used only in the context which permits sleeping */
> - if (gfpflags_allow_blocking(gfp)) {
> + /*
> + * CMA can be used only in the context which permits sleeping.
> + * Since addresses within one PAGE are always contiguous, skip
> + * CMA allocation for a single page to save CMA reserved space
> + * unless DMA_ATTR_FORCE_CONTIGUOUS is flagged.
> + */
> + if (gfpflags_allow_blocking(gfp) &&
> + (count > 1 || attrs & DMA_ATTR_FORCE_CONTIGUOUS)) {
And my other concern is that this skips allocating from the per-device
pool, which drivers might rely on. To be honest I'm not sure there is
much of a point in the per-device CMA pool vs the traditional per-device
coherent pool, but I'd rather change that behavior in a clearly documented
commit with intentions rather as a side effect from a random optimization.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area
2019-02-04 8:23 ` Christoph Hellwig
@ 2019-02-05 23:05 ` Nicolin Chen
2019-02-06 7:07 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2019-02-05 23:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: m.szyprowski, robin.murphy, vdumpa, iommu, linux-kernel
Hi Christoph
On Mon, Feb 04, 2019 at 09:23:07AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 15, 2019 at 01:51:40PM -0800, Nicolin Chen wrote:
> > The addresses within a single page are always contiguous, so it's
> > not so necessary to allocate one single page from CMA area. Since
> > the CMA area has a limited predefined size of space, it might run
> > out of space in some heavy use case, where there might be quite a
> > lot CMA pages being allocated for single pages.
> >
> > This patch tries to skip CMA allocations of single pages and lets
> > them go through normal page allocations unless the allocation has
> > a DMA_ATTR_FORCE_CONTIGUOUS attribute. This'd save some resources
> > in the CMA area for further more CMA allocations, and it can also
> > reduce CMA fragmentations resulted from trivial allocations.
>
> That DMA_ATTR_FORCE_CONTIGUOUS flag does not make sense. A single
> page allocation is per defintion always contigous.
>
> > again:
> > - /* CMA can be used only in the context which permits sleeping */
> > - if (gfpflags_allow_blocking(gfp)) {
> > + /*
> > + * CMA can be used only in the context which permits sleeping.
> > + * Since addresses within one PAGE are always contiguous, skip
> > + * CMA allocation for a single page to save CMA reserved space
> > + * unless DMA_ATTR_FORCE_CONTIGUOUS is flagged.
> > + */
> > + if (gfpflags_allow_blocking(gfp) &&
> > + (count > 1 || attrs & DMA_ATTR_FORCE_CONTIGUOUS)) {
>
> And my other concern is that this skips allocating from the per-device
> pool, which drivers might rely on.
Actually Robin had the same concern at v1 and suggested that we could
always use DMA_ATTR_FORCE_CONTIGUOUS to enforce into per-device pool.
> To be honest I'm not sure there is
> much of a point in the per-device CMA pool vs the traditional per-device
> coherent pool, but I'd rather change that behavior in a clearly documented
> commit with intentions rather as a side effect from a random optimization.
Hmm..sorry, I don't really follow this suggestion. Is it possible for
you to make it clear that what should I do for the change?
Thanks
Nicolin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area
2019-02-05 23:05 ` Nicolin Chen
@ 2019-02-06 7:07 ` Christoph Hellwig
2019-02-07 2:28 ` Nicolin Chen
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-02-06 7:07 UTC (permalink / raw)
To: Nicolin Chen
Cc: Christoph Hellwig, m.szyprowski, robin.murphy, vdumpa, iommu,
linux-kernel
On Tue, Feb 05, 2019 at 03:05:30PM -0800, Nicolin Chen wrote:
> > And my other concern is that this skips allocating from the per-device
> > pool, which drivers might rely on.
>
> Actually Robin had the same concern at v1 and suggested that we could
> always use DMA_ATTR_FORCE_CONTIGUOUS to enforce into per-device pool.
That is both against the documented behavior of DMA_ATTR_FORCE_CONTIGUOUS
and doesn't help existing drivers that specify their CMA area in DT.
> > To be honest I'm not sure there is
> > much of a point in the per-device CMA pool vs the traditional per-device
> > coherent pool, but I'd rather change that behavior in a clearly documented
> > commit with intentions rather as a side effect from a random optimization.
>
> Hmm..sorry, I don't really follow this suggestion. Is it possible for
> you to make it clear that what should I do for the change?
Something like this (plus proper comments):
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index b2a87905846d..789d734f0f77 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -192,10 +192,19 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
unsigned int align, bool no_warn)
{
+ struct cma *cma;
+
if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;
- return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
+ if (dev && dev->cma_area)
+ cma = dev->cma_area;
+ else if (count > PAGE_SIZE)
+ cma = dma_contiguous_default_area;
+ else
+ return NULL;
+
+ return cma_alloc(cma, count, align, no_warn);
}
/**
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area
2019-02-06 7:07 ` Christoph Hellwig
@ 2019-02-07 2:28 ` Nicolin Chen
2019-02-07 5:37 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2019-02-07 2:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: m.szyprowski, robin.murphy, vdumpa, iommu, linux-kernel
Hi Christoph,
On Wed, Feb 06, 2019 at 08:07:26AM +0100, Christoph Hellwig wrote:
> On Tue, Feb 05, 2019 at 03:05:30PM -0800, Nicolin Chen wrote:
> > > And my other concern is that this skips allocating from the per-device
> > > pool, which drivers might rely on.
> >
> > Actually Robin had the same concern at v1 and suggested that we could
> > always use DMA_ATTR_FORCE_CONTIGUOUS to enforce into per-device pool.
>
> That is both against the documented behavior of DMA_ATTR_FORCE_CONTIGUOUS
> and doesn't help existing drivers that specify their CMA area in DT.
OK. I will drop it.
> > > To be honest I'm not sure there is
> > > much of a point in the per-device CMA pool vs the traditional per-device
> > > coherent pool, but I'd rather change that behavior in a clearly documented
> > > commit with intentions rather as a side effect from a random optimization.
> >
> > Hmm..sorry, I don't really follow this suggestion. Is it possible for
> > you to make it clear that what should I do for the change?
>
> Something like this (plus proper comments):
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index b2a87905846d..789d734f0f77 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -192,10 +192,19 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
> struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
> unsigned int align, bool no_warn)
> {
> + struct cma *cma;
> +
> if (align > CONFIG_CMA_ALIGNMENT)
> align = CONFIG_CMA_ALIGNMENT;
>
> - return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
> + if (dev && dev->cma_area)
> + cma = dev->cma_area;
> + else if (count > PAGE_SIZE)
> + cma = dma_contiguous_default_area;
> + else
> + return NULL;
So we will keep allocating single pages in dev->cma_area if it's
present, in order to address your previous concern?
Thanks
Nicolin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] dma-direct: do not allocate a single page from CMA area
2019-02-07 2:28 ` Nicolin Chen
@ 2019-02-07 5:37 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-02-07 5:37 UTC (permalink / raw)
To: Nicolin Chen
Cc: Christoph Hellwig, m.szyprowski, robin.murphy, vdumpa, iommu,
linux-kernel
On Wed, Feb 06, 2019 at 06:28:49PM -0800, Nicolin Chen wrote:
> So we will keep allocating single pages in dev->cma_area if it's
> present, in order to address your previous concern?
Yes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-07 5:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 21:51 [PATCH v2] dma-direct: do not allocate a single page from CMA area Nicolin Chen
2019-02-04 8:23 ` Christoph Hellwig
2019-02-05 23:05 ` Nicolin Chen
2019-02-06 7:07 ` Christoph Hellwig
2019-02-07 2:28 ` Nicolin Chen
2019-02-07 5:37 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).