On Tue, Oct 15, 2024 at 8:42 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > > On 15.10.24 16:59, Suren Baghdasaryan wrote: > > On Tue, Oct 15, 2024 at 12:32 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > >> > >> On 15.10.24 01:53, John Hubbard wrote: > >>> On 10/14/24 4:48 PM, Yosry Ahmed wrote: > >>>> On Mon, Oct 14, 2024 at 1:37 PM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote: > >>>>> > >>>>> Add CONFIG_PGALLOC_TAG_USE_PAGEFLAGS to store allocation tag > >>>>> references directly in the page flags. This eliminates memory > >>>>> overhead caused by page_ext and results in better performance > >>>>> for page allocations. > >>>>> If the number of available page flag bits is insufficient to > >>>>> address all kernel allocations, profiling falls back to using > >>>>> page extensions with an appropriate warning to disable this > >>>>> config. > >>>>> If dynamically loaded modules add enough tags that they can't > >>>>> be addressed anymore with available page flag bits, memory > >>>>> profiling gets disabled and a warning is issued. > >>>> > >>>> Just curious, why do we need a config option? If there are enough bits > >>>> in page flags, why not use them automatically or fallback to page_ext > >>>> otherwise? > >>> > >>> Or better yet, *always* fall back to page_ext, thus leaving the > >>> scarce and valuable page flags available for other features? > >>> > >>> Sorry Suren, to keep coming back to this suggestion, I know > >>> I'm driving you crazy here! But I just keep thinking it through > >>> and failing to see why this feature deserves to consume so > >>> many page flags. > >> > >> My 2 cents: there is nothing wrong about consuming unused page flags in > >> a configuration. No need to let them stay unused in a configuration :) > >> > >> The real issue starts once another feature wants to make use of some of > >> them ... in such configuration there would be less available for > >> allocation tags and the performance of allocations tags might > >> consequently get worse again. > > > > Thanks for the input and indeed this is the case. If this happens, we > > will get a warning telling us that page flags could not be used and > > page_ext will be used instead. I think that's the best I can do given > > that page flag bits is a limited resource. > > Right, I think what John is concerned about (and me as well) is that > once a new feature really needs a page flag, there will be objection > like "no you can't, we need them for allocation tags otherwise that > feature will be degraded". I do understand your concern but IMHO the possibility of degrading a feature should not be a reason to always operate at degraded capacity (which is what we have today). If one is really concerned about possible future regression they can set CONFIG_PGALLOC_TAG_USE_PAGEFLAGS=n and keep what we have today. That's why I'm strongly advocating that we do need CONFIG_PGALLOC_TAG_USE_PAGEFLAGS so that the user has control over how this scarce resource is used. > > So a "The Lord has given, and the Lord has taken away!" mentality might > be required when consuming that many scarce resources, meaning, as long > as they are actually unused, use them, but it should not block other > features that really need them. I agree and I think that's what I implemented here. If there are enough page flag bits we use them, otherwise we automatically fall back to page_ext. > > Does that make sense? Absolutely and thank you all for the feedback. > > -- > Cheers, > > David / dhildenb >
