On Sat, 30 May 2026 22:19:32 +0000, Pasha Tatashin <[email protected]> wrote: > diff --git a/include/linux/kho_block.h b/include/linux/kho_block.h > new file mode 100644 > index 000000000000..5e6b87b1befa > --- /dev/null > +++ b/include/linux/kho_block.h > @@ -0,0 +1,79 @@ > [ ... skip 19 lines ... ] > + struct list_head list; > + struct kho_block_header_ser *ser; > +}; > + > +/** > + * struct kho_block_set - A set of blocks that belong to the same object.
"same object" sounds off to me. The blocks belong to the same module? user? Thoughts? > + * @blocks: The list of serialization blocks (struct kho_block). > + * @nblocks: The number of allocated serialization blocks. > + * @head_pa: Physical address of the first block header. > + * @entry_size: The size of each entry in the blocks. I think it's "... entry in a block" > [ ... skip 42 lines ... ] > + > +void kho_block_it_init(struct kho_block_it *it, struct kho_block_set *bs); > +void *kho_block_it_next(struct kho_block_it *it); > +void *kho_block_it_read(struct kho_block_it *it); > +void *kho_block_it_prev(struct kho_block_it *it); > +void kho_block_it_finalize(struct kho_block_it *it); These operate on block sets, should be reflected in the names. Can be kho_blocks_ to avoid too long names. > > diff --git a/kernel/liveupdate/kho_block.c b/kernel/liveupdate/kho_block.c > new file mode 100644 > index 000000000000..a4e650af946f > --- /dev/null > +++ b/kernel/liveupdate/kho_block.c > @@ -0,0 +1,384 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * Copyright (c) 2026, Google LLC. > + * Pasha Tatashin <[email protected]> > + */ > + > +/** > + * DOC: KHO Serialization Blocks > + * > + * KHO provides a mechanism to preserve stateful data across a kexec handover > + * by serializing it into memory blocks. This file provides the common "This file" does not look good in HTML docs. > [ ... skip 15 lines ... ] > + > +/* > + * Safeguard limit for the number of serialization blocks. This is used to > + * prevent infinite loops and excessive memory allocation in case of memory > + * corruption in the preserved state. > + */ Can you add how much memory it is and how many entries with, say, 4 u64 it can accommodate? > [ ... skip 13 lines ... ] > +{ > + if (unlikely(!bs->count_per_block)) { > + bs->count_per_block = (KHO_BLOCK_SIZE - > + sizeof(struct kho_block_header_ser)) / > + bs->entry_size; > + WARN_ON(!bs->count_per_block); Don't you want to set count_per_block in _init()? > [ ... skip 29 lines ... ] > + if (!block) > + return -ENOMEM; > + > + block->ser = ser; > + last = list_last_entry_or_null(&bs->blocks, struct kho_block, list); > + list_add_tail(&block->list, &bs->blocks); No locks? > [ ... skip 12 lines ... ] > + * @bs: The block set. > + * @count: The current number of entries. > + * > + * This function handles the dynamic expansion of a block set. It allocates > + * and links a new serialization block if the provided entry count matches > + * the current total capacity of the set. This is a weird semantics for a generic API. I'd expect _grow() would add count - current_count blocks. > [ ... skip 25 lines ... ] > +} > + > +/** > + * kho_block_shrink - Conditionally destroy the last block in a block set. > + * @bs: The block set. > + * @count: The current number of entries across all blocks. Maybe ... of valid entries? > + * > + * This function checks if the last block in the set is redundant based on > the > + * total entry count and the capacity of the preceding blocks. If the entry > + * count can be accommodated by the blocks that come before the last one, the > + * last block is destroyed and removed from the set. This should mention that it's the caller responsibility to ensure that entries are removed in the right order. > [ ... skip 49 lines ... ] > + > + fast = phys_to_virt(fast->next); > + slow = phys_to_virt(slow->next); > + > + if (slow == fast) { > + pr_err("Cyclic list detected\n"); Maybe "block set is corrupted"? > + return false; > + } > + } > + > + return true; > +} > + > +/** > + * kho_block_restore - Restore a block set from a physical address. > + * @bs: The block set to restore. > + * @head_pa: Physical address of the first block header. I'd mention that the block set should be allocated and initialized > [ ... skip 10 lines ... ] > + bs->incoming = true; > + if (!head_pa) > + return 0; > + > + bs->head_pa = head_pa; > + if (!kho_cyclic_blocks_check(bs)) { if (kho_block_set_cyclic()) reads nicer IMO > [ ... skip 87 lines ... ] > +{ > + if (!it->block) > + return NULL; > + > + if (it->i == kho_block_count_per_block(it->bs)) { > + it->block->ser->count = it->i; Why iterator updates ser->count? > + if (list_is_last(&it->block->list, &it->bs->blocks)) > + return NULL; > + it->block = list_next_entry(it->block, list); > + it->i = 0; > + } > + > + return (void *)(it->block->ser + 1) + (it->i++ * it->bs->entry_size); In a month we'll need an LLM's help to understand what it does. > +} > + > +/** > + * kho_block_it_read - Return the next entry slot for reading. > + * @it: The block iterator. And what is the conceptual difference between this and _it_next()? > [ ... skip 49 lines ... ] > + * @it: The block iterator. > + */ > +void kho_block_it_finalize(struct kho_block_it *it) > +{ > + if (it->block) > + it->block->ser->count = it->i; So, it looks like the intention of _it_next is for write, and this ends a write iteration. I think the names should be adjusted to make it clearer. -- Sincerely yours, Mike.
