On 30.04.20 18:33, Eric W. Biederman wrote: > David Hildenbrand <david@xxxxxxxxxx> writes: > >> On 30.04.20 17:38, Eric W. Biederman wrote: >>> David Hildenbrand <david@xxxxxxxxxx> writes: >>> >>>> Some devices/drivers that add memory via add_memory() and friends (e.g., >>>> dax/kmem, but also virtio-mem in the future) don't want to create entries >>>> in /sys/firmware/memmap/ - primarily to hinder kexec from adding this >>>> memory to the boot memmap of the kexec kernel. >>>> >>>> In fact, such memory is never exposed via the firmware memmap as System >>>> RAM (e.g., e820), so exposing this memory via /sys/firmware/memmap/ is >>>> wrong: >>>> "kexec needs the raw firmware-provided memory map to setup the >>>> parameter segment of the kernel that should be booted with >>>> kexec. Also, the raw memory map is useful for debugging. For >>>> that reason, /sys/firmware/memmap is an interface that provides >>>> the raw memory map to userspace." [1] >>>> >>>> We don't have to worry about firmware_map_remove() on the removal path. >>>> If there is no entry, it will simply return with -EINVAL. >>>> >>>> [1] >>>> https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-firmware-memmap >>> >>> >>> You know what this justification is rubbish, and I have previously >>> explained why it is rubbish. >> >> Actually, no, I don't think it is rubbish. See patch #3 and the cover >> letter why this is the right thing to do *for special memory*, *not >> ordinary DIMMs*. >> >> And to be quite honest, I think your response is a little harsh. I don't >> recall you replying to my virtio-mem-related comments. >> >>> >>> Nacked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> >>> >>> This needs to be based on weather the added memory is ultimately normal >>> ram or is something special. >> >> Yes, that's what the caller are expected to decide, see patch #3. >> >> kexec should try to be as closely as possible to a real reboot - IMHO. > > That is very fuzzy in terms of hotplug memory. The kexec'd kernel > should see the hotplugged memory assuming it is ordinary memory. > > But kexec is not a reboot although it is quite similar. Kexec is > swapping one running kernel and it's state for another kernel without > rebooting. I agree (especially regarding the arm64 DIMM hotplug discussion). However, for the two cases a) dax/kmem b) virtio-mem We really want to let the driver take back control and figure out "what to do with the memory". > >>> Justifying behavior by documentation that does not consider memory >>> hotplug is bad thinking. >> >> Are you maybe confusing this patch series with the arm64 approach? This >> is not about ordinary hotplugged DIMMs. > > I think I am. > > My challenge is that I don't see anything in the description that says > this isn't about ordinary hotplugged DIMMs. All I saw was hotplug > memory. I'm sorry if that was confusing, I tried to stress that kmem and virtio-mem is special in the description. I squeezed a lot of that information into the cover letter and into patch #3. > > If the class of memory is different then please by all means let's mark > it differently in struct resource so everyone knows it is different. > But that difference needs to be more than hotplug. > > That difference needs to be the hypervisor loaned us memory and might > take it back at any time, or this memory is persistent and so it has > these different characteristics so don't use it as ordinary ram. Yes, and I think kmem took an excellent approach of explicitly putting that "System RAM" into a resource hierarchy. That "System RAM" won't show up as a root node under /proc/iomem (see patch #3), which already results in kexec-tools to treat it in a special way. I am thinking about doing the same for virtio-mem. > > That information is also useful to other people looking at the system > and seeing what is going on. > > Just please don't muddle the concepts, or assume that whatever subset of > hotplug memory you are dealing with is the only subset. I can certainly rephrase the subject/description/comment, stating that this is not to be used for ordinary hotplugged DIMMs - only when the device driver is under control to decide what to do with that memory - especially when kexec'ing. (previously, I called this flag MHP_DRIVER_MANAGED, but I think MHP_NO_FIRMWARE_MEMMAP is clearer, we just need a better description) Would that make it clearer? Thanks! -- Thanks, David / dhildenb
