dma-coherent: add support for multi coherent rmems per dev

Add support for multiple coherent rmems per device. This patch replaces original dma_mem with dma_mems list in device structure to store multiple rmems.

These multiple rmems can be assigned to the device one by one by of_reserved_mem_device_init_by_idx() with the memory-region declaration in device tree as below and store the rmem to the dma_mems list.

	device1@0 {
		...
		memory-region = <&reserved_mem0>, <&reserved_mem1>;
		...
	};

When driver tries to allocate memory from the rmems, looks for the first available rmem and allocates the memory from this rmem.

Then if driver removed, of_reserved_mem_device_release() needs to be invoked to release all the rmems assigned to the device.

kernel/dma/coherent.c +1 -1

 	struct dma_coherent_mem *mem;
-	int ret;
+	int retval;
 
 	mem = dma_init_coherent_memory(phys_addr, device_addr, size, false);

int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,

kernel/dma/coherent.c +1 -1

 		dma_addr_t *dma_handle, void **ret)
 {
-	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+	struct dma_coherent_mem *mem_tmp;

kernel/dma/coherent.c +1 -0

+	}
 
 	return 1;
 
 }

kernel/dma/coherent.c +2 -1

+	}
 
-	return __dma_release_from_coherent(mem, order, vaddr);
+	return retval;
 }

Quoted 20 lines · Howard Yen

Add support for multiple coherent rmems per device. This patch replaces original dma_mem with dma_mems list in device structure to store multiple rmems.

These multiple rmems can be assigned to the device one by one by of_reserved_mem_device_init_by_idx() with the memory-region declaration in device tree as below and store the rmem to the dma_mems list.

      device1@0 {
              ...
              memory-region = <&reserved_mem0>, <&reserved_mem1>;
              ...
      };

When driver tries to allocate memory from the rmems, looks for the first available rmem and allocates the memory from this rmem.

Then if driver removed, of_reserved_mem_device_release() needs to be invoked to release all the rmems assigned to the device.

...

Quoted 5 lines

      struct dma_coherent_mem *mem;

kernel/dma/coherent.c +1 -1

-     int ret;
+     int retval;
 
      mem = dma_init_coherent_memory(phys_addr, device_addr, size, false);

kernel/dma/coherent.c int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr, · line 123

Quoted 10 lines

kernel/dma/coherent.c +3 -3

      if (IS_ERR(mem))
              return PTR_ERR(mem);
 
-     ret = dma_assign_coherent_memory(dev, mem);
-     if (ret)
+     retval = dma_assign_coherent_memory(dev, mem);
+     if (retval)
              _dma_release_coherent_memory(mem);
-     return ret;
+     return retval;

This is unrelated change.

But why? Do you have retval in the _existing_ code elsewhere?

...

Quoted 5 lines

int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
dma_addr_t *dma_handle, void **ret)
{

kernel/dma/coherent.c +1 -1

-     struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+     struct dma_coherent_mem *mem_tmp;

kernel/dma/coherent.c static void *__dma_alloc_from_coherent(struct device *dev, · line 196

Quoted 10 lines

kernel/dma/coherent.c +5 -2

 
-     if (!mem)
+     if (list_empty(&dev->dma_mems))
              return 0;
 
-     *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
+     list_for_each_entry(mem_tmp, &dev->dma_mems, node) {
+             *ret = __dma_alloc_from_coherent(dev, mem_tmp, size, dma_handle);
+             if (*ret)
+                     break;

This bails out on the first success. Moreover, if one calls this function again, it will rewrite the existing allocation. Is this all expected?

OTOH, if you add multiple entries and bailing out on error condition it should be clear if the previous allocations have to be released.

Quoted 6 lines

+ }

      return 1;

}

...

kernel/dma/coherent.c static int __dma_release_from_coherent(struct dma_coherent_mem *mem, · line 238

Quoted 10 lines

int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr) {

kernel/dma/coherent.c +7 -1

-     struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+     struct dma_coherent_mem *mem_tmp;
+     int retval = 0;
+
+     list_for_each_entry(mem_tmp, &dev->dma_mems, node) {
+             retval = __dma_release_from_coherent(mem_tmp, order, vaddr);
+             if (retval == 1)
+                     break;

Same Q here.

Quoted 5 lines

kernel/dma/coherent.c +2 -1

+     }
 
-     return __dma_release_from_coherent(mem, order, vaddr);
+     return retval;
 }

...

kernel/dma/coherent.c static int __dma_mmap_from_coherent(struct dma_coherent_mem *mem, · line 290

Quoted 12 lines

int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
void *vaddr, size_t size, int *ret)
{

kernel/dma/coherent.c +6 -2

-     struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+     struct dma_coherent_mem *mem_tmp;
+     int retval = 0;
 
-     return __dma_mmap_from_coherent(mem, vma, vaddr, size, ret);
+     list_for_each_entry(mem_tmp, &dev->dma_mems, node) {
+             retval = __dma_mmap_from_coherent(mem_tmp, vma, vaddr, size, ret);
+             if (retval == 1)
+                     break;

And here.

kernel/dma/coherent.c static int __dma_mmap_from_coherent(struct dma_coherent_mem *mem, · line 294

Quoted 4 lines

kernel/dma/coherent.c +3 -0

+     }
+
+     return retval;
 }

...

With the above Q in mind, here is another one: Why can't we allocate all at once?

--
With Best Regards,
Andy Shevchenko

Add support for multiple coherent rmems per device.

Quoted 1 line · Howard Yen

Add support for multiple coherent rmems per device.

Why?

I tried to upload the patch to support multiple coherent rmems per device because in some system, the primary memory space for the device might be limited, so that add multiple coherent rmems support per device to satisfy the scenario.

Quoted 4 lines · Howard Yen

I tried to upload the patch to support multiple coherent rmems per device because in some system, the primary memory space for the device might be limited, so that add multiple coherent rmems support per device to satisfy the scenario.

I'm not sure what the means.

If you have non-trivial patches you really need to explain why you're doing in detail as you need to convince maintainers that it is useful.

The reason why I tried to propose this patch is that in the system I'm working on, where the driver utilizes the coherent reserved memory in the subsystem for DMA, which has limited memory space as its primary usage. During the execution of the driver, there is a possibility of encountering memory depletion scenarios with the primary one.

To address this issue, I tried to create a patch that enables the coherent reserved memory driver to support multiple coherent reserved memory regions per device. This modification aims to provide the driver with the ability to search for memory from a secondary region if the primary memory is exhausted, and so on.

Quoted 11 lines · Howard Yen

The reason why I tried to propose this patch is that in the system I'm working on, where the driver utilizes the coherent reserved memory in the subsystem for DMA, which has limited memory space as its primary usage. During the execution of the driver, there is a possibility of encountering memory depletion scenarios with the primary one.

To address this issue, I tried to create a patch that enables the coherent reserved memory driver to support multiple coherent reserved memory regions per device. This modification aims to provide the driver with the ability to search for memory from a secondary region if the primary memory is exhausted, and so on.

This all seems pretty vague. Can you point to your driver submission and explain why it can't just use a larger region instead of multiple smaller ones?

Quoted 14 lines · Christoph Hellwig

Quoted 11 lines · Howard Yen

The reason why I tried to propose this patch is that in the system I'm working on, where the driver utilizes the coherent reserved memory in the subsystem for DMA, which has limited memory space as its primary usage. During the execution of the driver, there is a possibility of encountering memory depletion scenarios with the primary one.

To address this issue, I tried to create a patch that enables the coherent reserved memory driver to support multiple coherent reserved memory regions per device. This modification aims to provide the driver with the ability to search for memory from a secondary region if the primary memory is exhausted, and so on.

Quoted 4 lines

This all seems pretty vague. Can you point to your driver submission and explain why it can't just use a larger region instead of multiple smaller ones?

The reason why it needs multiple regions is that in my system there is an always-on subsystem which includes a small size memory, and several functions need to run and occupy the memory from the small memory if they need to run on the always-on subsystem. These functions must allocate the memory from the small memory region, so that they can get benefit from the always-on subsystem. So the small memory is split for multiple functions which are satisfied with their generic use cases.

But in specific use cases, they required more memory than their pre-allocated memory region, so I tried to propose this patch to give it the ability to get the memory from another larger memory to solve the issue.

I'll upload the next version to show the modification in the driver.

---
Best Regards,

Howard

Quoted 20 lines · Howard Yen

Quoted 14 lines · Christoph Hellwig

Quoted 11 lines · Howard Yen

The reason why I tried to propose this patch is that in the system I'm working on, where the driver utilizes the coherent reserved memory in the subsystem for DMA, which has limited memory space as its primary usage. During the execution of the driver, there is a possibility of encountering memory depletion scenarios with the primary one.

To address this issue, I tried to create a patch that enables the coherent reserved memory driver to support multiple coherent reserved memory regions per device. This modification aims to provide the driver with the ability to search for memory from a secondary region if the primary memory is exhausted, and so on.

Quoted 4 lines

This all seems pretty vague. Can you point to your driver submission and explain why it can't just use a larger region instead of multiple smaller ones?

Quoted 7 lines

The reason why it needs multiple regions is that in my system there is an always-on subsystem which includes a small size memory, and several functions need to run and occupy the memory from the small memory if they need to run on the always-on subsystem. These functions must allocate the memory from the small memory region, so that they can get benefit from the always-on subsystem. So the small memory is split for multiple functions which are satisfied with their generic use cases.

I don't really see how that aligns with what you've implemented, though. The coherent allocator doesn't have any notion of the caller's use-case, it's just going to allocate from wherever it happens to find space first. Thus even the calls which would somehow benefit from allocating from the "primary" region will have no way to guarantee that they will actually allocate from there if it's already been consumed by callers who didn't need that benefit but just happened to get there first.

Really that sounds like a case for having specific named memory-regions and managing them directly from the relevant driver, rather than trying to convince the generic dma-coherent abstraction to do things that don't really fit it. Otherwise I'd be slightly concerned that you're expecting to bake secret undocumented ABI into DMA API implementations where some particular order of allocations must guarantee a particular deterministic behaviour, which is really not something we want.

Thanks,
Robin.

Quoted 11 lines

But in specific use cases, they required more memory than their pre-allocated memory region, so I tried to propose this patch to give it the ability to get the memory from another larger memory to solve the issue.

I'll upload the next version to show the modification in the driver.

---
Best Regards,

Howard

Quoted 1 line · Howard Yen

Add support for multiple coherent rmems per device.

Why?