On Mon, Oct 27, 2025 at 7:02 PM David Matlack <[email protected]> wrote: > > On Mon, Oct 27, 2025 at 3:56 PM David Matlack <[email protected]> wrote: > > > > On Mon, Oct 20, 2025 at 5:09 PM Pasha Tatashin > > <[email protected]> wrote: > > > > > -static void *xa_load_or_alloc(struct xarray *xa, unsigned long index, > > > size_t sz) > > > +static void *xa_load_or_alloc(struct xarray *xa, unsigned long index) > > > { > > > void *res = xa_load(xa, index); > > > > > > if (res) > > > return res; > > > > > > - void *elm __free(kfree) = kzalloc(sz, GFP_KERNEL); > > > + void *elm __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > > > > > > if (!elm) > > > return ERR_PTR(-ENOMEM); > > > > > > - if (WARN_ON(kho_scratch_overlap(virt_to_phys(elm), sz))) > > > + if (WARN_ON(kho_scratch_overlap(virt_to_phys(elm), PAGE_SIZE))) > > > return ERR_PTR(-EINVAL); > > > > Reading xa_load_or_alloc() is a bit confusing now. > > > > It seems very generic (returns a void *) but now hard-codes a size > > (PAGE_SIZE). You have to look at the caller to see it is allocating > > for a struct kho_mem_phys_bits, and then at the definition of struct > > kho_mem_phys_bits to see the static_assert() that this struct is > > always PAGE_SIZE. > > > > I would either keep letting the caller passing in size (if you think > > this code is going to be re-used) or just commit to making > > xa_load_or_alloc() specific to kho_mem_phys_bits. e.g. Change the > > return type to struct kho_mem_phys_bits * and use sizeof() instead of > > PAGE_SIZE. > > I see that you replace kzalloc() with get_zeroed_page() in the next > patch. So the latter option is probably better, and maybe move static > assert down here and use BUILD_BUG_ON()? That way readers can easily > see that we are allocating for struct kho_mem_phys_bits *and* that > that struct is guaranteed to be PAGE_SIZE'd.
The size is verified at build time via: +static_assert(sizeof(struct kho_mem_phys_bits) == PAGE_SIZE);
