Possible bug when using dmam_alloc_coherent in conjonction with of_reserved_mem_device_release

Olivier Dautricourt
2021-05-21T18:15:59Z

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)

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