Possible bug when using dmam_alloc_coherent in conjonction with of_reserved_mem_device_release

[Olivier Dautricourt <olivier.dautricourt@xxxxxxxxxx>]

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