On Wed, Feb 12, 2025 at 01:43:03PM -0400, Jason Gunthorpe wrote: > On Wed, Feb 12, 2025 at 06:39:06PM +0200, Mike Rapoport wrote: > > > As I've mentioned off-list earlier, KHO in its current form is the lowest > > level of abstraction for state preservation and it is by no means is > > intended to provide complex drivers with all the tools necessary. > > My point, is I think it is the wrong level of abstraction and the > wrong FDT schema. It does not and cannot solve the problems we know we > will have, so why invest anything into that schema? Preserving a lot of random pages spread all over the place will be a problem no matter what. With kho_preserve_folio() the users will still need to save physical address of that folio somewhere, be it FDT or some binary structure that FDT will point to. So either instead of "mem" properties we'll have "addresses" property or a pointer to yet another page that should be preserved and, by the way, "mem" may come handy in this case :) I don't see how the "mem" property contradicts future extensions and for simple use cases it is already enough. The simple reserve_mem use case in this patchset indeed does not represent the complexity of a driver, but it's still useful, at least for the ftrace folks. And reserve_mem is just fine with "mem" property. > I think the scratch system is great, and an amazing improvement over > past version. Upgrade the memory preservation to match and it will be > really good. > > > What you propose is a great optimization for memory preservation mechanism, > > and additional and very useful abstraction layer on top of "basic KHO"! > > I do not see this as a layer on top, I see it as fundamentally > replacing the memory preservation mechanism with something more > scalable. There are two parts to the memory preservation: making sure the preserved pages don't make it to the free lists and than restoring struct page/folio/memdesc so that the pages will look the same way as when they were allocated. For the first part we must memblock_reserve(addr, size) for every preserved range before memblock releases memory to the buddy. I did an experiment and preserved 1GiB of random order-0 pages and measured time required to reserve everything in memblock. kho_deserialize() you suggested slightly outperformed kho_init_reserved_pages() that parsed a single "mem" property containing an array of <addr, size> pairs. For more random distribution of orders and more deep FDT the difference or course would be higher, but still both options sucked relatively to a maple tree serialized similarly to your tracker xarray. For the restoration of the struct folio for multiorder folios the tracker xarray is a really great fit, but again, it does not contradict having "mem" properties. And the restoration of struct folio does not have to happen very early, so we'd probably want to run it in parallel, somewhat like deferred initialization of struct page. > > But I think it will be easier to start with something *very simple* and > > probably suboptimal and then extend it rather than to try to build complex > > comprehensive solution from day one. > > But why? Just do it right from the start? I spent like a hour > sketching that, the existing preservation code is also very simple, > why not just fix it right now? As I see it, we can have both. "mem" property for simple use cases, or as a partial solution for complex use cases and tracker you proposed for preserving the order of the folios. And as another optimization we may want a maple tree for coalescing as much as possible to reduce amount of memblock_reserve() calls. > Jason -- Sincerely yours, Mike.
