On 8/2/23 9:24 PM, David Hildenbrand wrote: > On 02.08.23 17:50, Michal Hocko wrote: >> On Wed 02-08-23 10:15:04, Aneesh Kumar K V wrote: >>> On 8/1/23 4:20 PM, Michal Hocko wrote: >>>> On Tue 01-08-23 14:58:29, Aneesh Kumar K V wrote: >>>>> On 8/1/23 2:28 PM, Michal Hocko wrote: >>>>>> On Tue 01-08-23 10:11:16, Aneesh Kumar K.V wrote: >>>>>>> Allow updating memmap_on_memory mode after the kernel boot. Memory >>>>>>> hotplug done after the mode update will use the new mmemap_on_memory >>>>>>> value. >>>>>> >>>>>> Well, this is a user space kABI extension and as such you should spend >>>>>> more words about the usecase. Why we could live with this static and now >>>>>> need dynamic? >>>>>> >>>>> >>>>> This enables easy testing of memmap_on_memory feature without a kernel reboot. >>>> >>>> Testing alone is rather weak argument to be honest. >>>> >>>>> I also expect people wanting to use that when they find dax kmem memory online >>>>> failing because of struct page allocation failures[1]. User could reboot back with >>>>> memmap_on_memory=y kernel parameter. But being able to enable it via sysfs makes >>>>> the feature much more useful. >>>> >>>> Sure it can be useful but that holds for any feature, right. The main >>>> question is whether this is worth maintaing. The current implementation >>>> seems rather trivial which is an argument to have it but are there any >>>> risks long term? Have you evaluated a potential long term maintenance >>>> cost? There is no easy way to go back and disable it later on without >>>> breaking some userspace. >>>> >>>> All that should be in the changelog! >>> >>> I updated it as below. >>> >>> mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter >>> >>> Allow updating memmap_on_memory mode after the kernel boot. Memory >>> hotplug done after the mode update will use the new mmemap_on_memory >>> value. >>> >>> It is now possible to test the memmap_on_memory feature easily without >>> the need for a kernel reboot. Additionally, for those encountering >>> struct page allocation failures while using dax kmem memory online, this >>> feature may prove useful. Instead of rebooting with the >>> memmap_on_memory=y kernel parameter, users can now enable it via sysfs, >>> which greatly enhances its usefulness. >> >> >> I do not really see a solid argument why rebooting is really a problem >> TBH. Also is the global policy knob really the right fit for existing >> hotplug usecases? In other words, if we really want to make >> memmap_on_memory more flexible would it make more sense to have it per >> memory block property instead (the global knob being the default if >> admin doesn't specify it differently). > > Per memory block isn't possible, due to the creation order. Also, I think it's not the right approach. > > I thought about driver toggles. At least for dax/kmem people are looking into that: > > https://lkml.kernel.org/r/20230801-vv-kmem_memmap-v3-2-406e9aaf5689@xxxxxxxxx > > Where that can also be toggled per device. > That still is dependent on the global memmap_on_memory enabled right? We could make the dax facility independent of the global feature toggle? -aneesh
