Le sam., mai 22 2021 at 13:42:54 +0200, Olivier Dautricourt
<olivier.dautricourt@xxxxxxxxxx> a écrit :
Hello Paul,
The 05/22/2021 11:28, Paul Cercueil wrote:
Hi Olivier,
Le ven., mai 21 2021 at 20:15:42 +0200, Olivier Dautricourt
<olivier.dautricourt@xxxxxxxxxx> a écrit :
> Hello all,
>
> I am facing a problem when using dmam_alloc_coherent (the managed
> version of dma_alloc_coherent) along with a device-specific
reserved
> memory
> region using the CMA.
>
> My observation is on a kernel 5.10.19 (arm), as i'm unable to test
> the exact
> same configuration on a newer kernel. However it seems that the
> relevent code
> did not change too much since, so i think it's still applicable.
>
>
> ....
> The issue:
>
> I declare a reserved region on my board such as:
>
> mydevice_reserved: linux,cma {
> compatible = "shared-dma-pool";
> reusable;
> size = <0x2400000>;
> };
>
> and start the kernel with cma=0, i want my region to be reserved
to
> my device.
>
> My driver basically does:
>
> probe(dev):
> of_reserved_mem_device_init(dev)
> dmam_alloc_coherent(...)
>
> release(dev):
> of_reserved_mem_device_release(dev)
You must make sure that whatever is allocated or initialized is
freed
or deinitialized in the reverse order, which is not what will happen
here: release(dev) will be called before the dev-managed cleanups.
To fix your issue, either use dma_alloc_coherent() and call
dma_free_coherent() in release(), or register
of_reserved_mem_device_release() as a dev-managed cleanup function
(which is what my driver does).
Cheers,
-Paul
as i was saying in my previous mail, i tried to register a devm action
to trigger of_reserved_mem_device_release on cleanup but it was still
called before dmam_alloc_coherent_memory's cleanup.
So my question is: How do you make sure that the managed cleanup
routines
are executed in the right order ?
And when exactly do you register the devm action?
If you register it right after of_reserved_mem_device_init() and before
dmam_alloc_coherent(), like in my driver, it should work fine (provided
your .release doesn't call of_reserved_mem_device_release() itself) and
you shouldn't have to do anything more than that.
-Paul
Should we have to care about that when using a managed
function that belongs to the core ?
Olivier
> On driver detach, of_reserved_mem_device_release will call
> rmem_cma_device_release which sets dev->cma_area = NULL;
> Then the manager will try to free the dma memory allocated in the
> probe:
>
> __free_from_contiguous -> dma_release_from_contiguous ->
> cma_release(dev_get_cma_area(dev), ...);
>
> Except that now dev_get_cma_area will return
> dma_contiguous_default_area
> which is null in my setup:
>
> static inline struct cma *dev_get_cma_area(struct device *dev)
> {
> if (dev && dev->cma_area) // dev->cma_area is null
> return dev->cma_area;
>
> return dma_contiguous_default_area; // null in my setup
> }
>
> and so cma_release will do nothing.
>
> bool cma_release(struct cma *cma, const struct page *pages,
unsigned
> int count)
> {
> unsigned long pfn;
>
> if (!cma || !pages) // cma is NULL
> return false;
>
> __free_from_contiguous will fail silently because it ignores
> dma_release_from_contiguous boolean result.
>
> The driver will be unable to load and allocate memory again
because
> the
> area allocated with dmam_alloc_coherent is not freed.
> ...
>
> So i started to look at drivers using both dmam_alloc_coherent and
> of_reserved_mem_device_release and found this driver:
> (gpu/drm/ingenic/ingenic-drm-drv.c).
> This is why i included the original author, Paul Cercueil, in the
> loop.
>
> Q:
>
> I noticed that Paul used devm_add_action_or_reset to trigger
> of_reserved_mem_device_release on driver detach, is this because
of
> this
> problem that we use a devm trigger here ?
>
> I tried to do the same in my driver, but rmem_cma_device_release
is
> still
> called before dmam_release, is there a way to force the order ?
>
> Is what i described a bug that needs fixing ?
>
>
> Thank you,
>
>
> Olivier
>
>
