Re: [PATCH v6 07/13] kho: add support for linked-block serialization

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On 06-03 12:05, Pasha Tatashin wrote:
> On 06-03 09:49, Mike Rapoport wrote:
> > On Wed, 03 Jun 2026 03:28:58 +0000, Pasha Tatashin <pasha.tatashin@soleen.com> wrote:
> > > diff --git a/include/linux/kho/abi/block.h b/include/linux/kho/abi/block.h
> > > new file mode 100644
> > > index 000000000000..8641c20b379b
> > > --- /dev/null
> > > +++ b/include/linux/kho/abi/block.h
> > > @@ -0,0 +1,56 @@
> > > [ ... skip 25 lines ... ]
> > > +#define _LINUX_KHO_ABI_BLOCK_H
> > > +
> > > +#include <asm/page.h>
> > > +#include <linux/types.h>
> > > +
> > > +#define KHO_BLOCK_ABI_COMPATIBLE	"kho-block-v1"
> > 
> > It's never used by block set and after looking at the following patches I
> > found that it's appended to LUO compatible string.
> > 
> > While this works for LUO, I think it should be kho_block_set_restore()
> > responsibility to verify the compatibility.
> 
> It should work for any component that relies on  kho_block. My proposal 
> is to use this method for other common KHO data structures (e.g.,  kho 
> vmalloc,  kho radix, future kho xarray). There is no need for them to 
> carry the compatibility string in their metadata, as whoever uses them 
> will include their compatibility string.
> 
> For now, reviewers will have to make sure that if the ABI header content 
> is changed, the compatibility string is updated.
> 
> > > diff --git a/kernel/liveupdate/kho_block.c b/kernel/liveupdate/kho_block.c
> > > new file mode 100644
> > > index 000000000000..4f147c308e6b
> > > --- /dev/null
> > > +++ b/kernel/liveupdate/kho_block.c
> > > @@ -0,0 +1,411 @@
> > > [ ... skip 121 lines ... ]
> > > +/**
> > > + * kho_block_set_grow - Expand the block set to accommodate the target count.
> > > + * @bs:    The block set.
> > > + * @count: The target number of valid entries to accommodate.
> > > + *
> > > + * Acts as a runtime notifier when new resources (such as files or sessions)
> > 
> > Not sure I understand what "runtime notifier" means in this context.
> 
> It came from discussion with Pratyush, but I think we are on the same 
> page what they are, and I will just remove this.
> 
> > 
> > > [ ... skip 11 lines ... ]
> > > +
> > > +	while (count > bs->nblocks * bs->count_per_block) {
> > > +		int err = kho_block_set_grow_one(bs);
> > > +
> > > +		if (err)
> > > +			return err;
> > 
> > This leaks memory if more than one block is added.
> > 
> > > [ ... skip 31 lines ... ]
> > > + * unregistered, allowing the block set to release and unallocate redundant
> > > + * preserved memory blocks. Checks if the last block in the set can be removed
> > > + * because the remaining entry count is fully accommodated by the preceding blocks.
> > > + *
> > > + * Note: It is the caller's responsibility to ensure that entries are removed
> > > + * in LIFO (last-in, first-out) order (the reverse order of their insertion).
> > 
> > I think "in LIFO order" is sufficient :)
> 
> Oh, I keep removing those :-)
> 
> > > [ ... skip 173 lines ... ]
> > > +		it->i = 0;
> > > +	}
> > > +
> > > +	entry = kho_block_entry(it, it->i++);
> > > +	it->block->ser->count = it->i;
> > > +	return entry;
> > 
> > This looks way better than the previous version :)
> > Thanks!
> 
> Thank you. I will send a new version of this patch as a reply to this 
> email to avoid cluttering the mailing list.

The patch is here:
https://lore.kernel.org/all/20260603130612.397948-1-pasha.tatashin@soleen.com/

I messed up in-reply-to field with wrong message-id.

Pasha

> 
> Pasha
> 
> > 
> > -- 
> > Sincerely yours,
> > Mike.
> >