On Wed, Sep 3, 2025 at 9:05 AM Thierry Reding <thierry.red...@gmail.com> wrote: > > On Tue, Sep 02, 2025 at 10:27:01AM -0700, Frank van der Linden wrote: > > On Tue, Sep 2, 2025 at 8:46 AM Thierry Reding <thierry.red...@gmail.com> > > wrote: > > > > > > From: Thierry Reding <tred...@nvidia.com> > > > > > > There is no technical reason why there should be a limited number of CMA > > > regions, so extract some code into helpers and use them to create extra > > > functions (cma_create() and cma_free()) that allow creating and freeing, > > > respectively, CMA regions dynamically at runtime. > > > > > > Note that these dynamically created CMA areas are treated specially and > > > do not contribute to the number of total CMA pages so that this count > > > still only applies to the fixed number of CMA areas. > > > > > > Signed-off-by: Thierry Reding <tred...@nvidia.com> > > > --- > > > include/linux/cma.h | 16 ++++++++ > > > mm/cma.c | 89 ++++++++++++++++++++++++++++++++++----------- > > > 2 files changed, 83 insertions(+), 22 deletions(-) > [...] > > I agree that supporting dynamic CMA areas would be good. However, by > > doing it like this, these CMA areas are invisible to the rest of the > > system. E.g. cma_for_each_area() does not know about them. It seems a > > bit inconsistent that there will now be some areas that are globally > > known, and some that are not. > > That was kind of the point of this experiment. When I started on this I > ran into the case where I was running out of predefined CMA areas and as > I went looking for ways on how to fix this, I realized that there's not > much reason to keep a global list of these areas. And even less reason > to limit the number of CMA areas to this predefined list. Very little > code outside of the core CMA code even uses this. > > There's one instance of cma_for_each_area() that I don't grok. There's > another early MMU fixup for CMA areas in 32-bit ARM that. Other than > that there's a few places where the total CMA page count is shown for > informational purposes and I don't know how useful that really is > because totalcma_pages doesn't really track how many pages are used for > CMA, but pages that could potentially be used for CMA. > > And that's about it. > > It seems like there are cases where we might really need to globally > know about some of these areas, specifically ones that are allocated > very early during boot and then used for very specific purposes. > > However, it seems to me like CMA is more universally useful than just > for these cases and I don't see the usefulness of tracking these more > generic uses. > > > I am being somewhat selfish here, as I have some WIP code that needs > > the global list :-) But I think the inconsistency is a more general > > point than just what I want (and the s390 code does use > > cma_for_each_area()). Maybe you could keep maintaining a global > > structure containing all areas? > > If it's really useful to be able to access all CMA areas, then we could > easily just add them all to a global linked list upon activation (we may > still want/need to keep the predefined list around for all those early > allocation cases). That way we'd get the best of both worlds. > > > What do you think are the chances of running out of the global count > > of areas? > > Well, I did run out of CMA areas during the early VPR testing because I > was initially testing with 16 areas and a different allocation scheme > that turned out to cause too many resizes in common cases. > > However, given that the default is 8 on normal systems (20 on NUMA) and > is configurable, it means that even with restricting this to 4 for VPR > doesn't always guarantee that all 4 are available. Again, yes, we could > keep bumping that number, but why not turn this into something a bit > more robust where nobody has to know or care about how many there are? > > > Also, you say that "these are treated specially and do not contribute > > to the number of total CMA pages". But, if I'm reading this right, you > > do call cma_activate_area(), which will do > > init_cma_reserved_pageblock() for each pageblock in it. Which adjusts > > the CMA counters for the zone they are in. But your change does not > > adjust totalcma_pages for dynamically created areas. That seems > > inconsistent, too. > > I was referring to just totalcma_pages that isn't impacted by these > dynamically allocated regions. This is, again, because I don't see why > that information would be useful. It's a fairly easy change to update > that value, so if people prefer that, I can add that. > > I don't see an immediate connection between totalcma_pages and > init_cma_reserved_pageblock(). I thought the latter was primarily useful > for making sure that the CMA pages can be migrated, which is still > critical for this use-case.
My comment was about statistics, they would be inconsistent after your change. E.g. currently, totalcma_pages is equal to the sum of CMA pages in each zone. But that would no longer be true, and applications / administrators looking at those statistics might see the inconsistency (between meminfo and vmstat) and wonder what's going on. It seems best to keep those numbers in sync. In general, I think it's fine to support dynamic allocation, and I agree with your arguments that it doesn't seem right to set the number of CMA areas via a config option. I would just like there to be a canonical way to find all CMA areas. - Frank
