On Thu, Apr 24, 2025 at 05:13:47PM -0700, John Stultz wrote: > On Thu, Apr 24, 2025 at 1:34 AM Maxime Ripard <mrip...@kernel.org> wrote: > > On Tue, Apr 22, 2025 at 12:19:39PM -0700, Jared Kangas wrote: > > > @@ -22,6 +22,7 @@ > > > #include <linux/slab.h> > > > #include <linux/vmalloc.h> > > > > > > +#define DEFAULT_CMA_NAME "default_cma" > > > > I appreciate this is kind of bikeshed-color territory, but I think "cma" > > would be a better option here. There's nothing "default" about it. > > I disagree. It very much is "default" as it's returning the > dma_contiguous_default_area.
My main concern here is that it's "default" as opposed to what, exactly? We have a single CMA allocator. We could have multiple buffer attributes, but then "cached_cma" would make more sense to me if we expect to have uncached CMA allocations at some point. > There can be multiple CMA areas, and out of tree, vendors do reserve > separate areas for specific purposes, exposing multiple CMA dmabuf > heaps. By "CMA areas", I guess you mean carved-out memory regions? If so, how is it relevant to userspace if we use CMA or any other implementation to expose a carved-out region, and thus that we carry that implemenattion detail in the name? > There have been patches to expose multiple CMA heaps, but with no > upstream drivers using those purpose specific regions, we haven't > taken them yet. > I do hope as the drivers that utilize these purpose focused heaps go > upstream, we can add that logic, so I think being specific that this > is default CMA is a good idea. If heaps names are supposed to carry the region it exposes, then it should be default_cma_region/area. If heap names are supposed to expose the allocator (but I don't think it's a good idea), it should be cma. If they are meant to carry all that plus some policy, cached_default_cma_region should be used. Either way, default_cma seems to me either too specific or not specific enough. And we should really document what the policy for those heaps are supposed to be. Maxime
signature.asc
Description: PGP signature
