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). -- Michal Hocko SUSE Labs
