[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+CK2bAKjU8PmfuwGJwwOyj3U5bYaterE_KQa9uVu+mmmLmmew@mail.gmail.com>
Date: Mon, 27 Oct 2025 20:03:13 -0400
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: David Matlack <dmatlack@...gle.com>
Cc: akpm@...ux-foundation.org, brauner@...nel.org, corbet@....net,
graf@...zon.com, jgg@...pe.ca, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-mm@...ck.org, masahiroy@...nel.org,
ojeda@...nel.org, pratyush@...nel.org, rdunlap@...radead.org, rppt@...nel.org,
tj@...nel.org, jasonmiu@...gle.com, skhawaja@...gle.com
Subject: Re: [PATCH v3 2/3] liveupdate: kho: Increase metadata bitmap size to PAGE_SIZE
On Mon, Oct 27, 2025 at 7:02 PM David Matlack <dmatlack@...gle.com> wrote:
>
> On Mon, Oct 27, 2025 at 3:56 PM David Matlack <dmatlack@...gle.com> wrote:
> >
> > On Mon, Oct 20, 2025 at 5:09 PM Pasha Tatashin
> > <pasha.tatashin@...een.com> 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);
Powered by blists - more mailing lists