Re: [PATCH] bootmem: Stop using page->index

[Matthew Wilcox <willy@infradead.org>]

On Thu, Sep 12, 2024 at 02:56:44PM -0700, Yosry Ahmed wrote:
> On Thu, Sep 12, 2024 at 2:51 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Thu, Sep 12, 2024 at 12:04:18PM -0700, Yosry Ahmed wrote:
> > > On Thu, Sep 12, 2024 at 11:56 AM Matthew Wilcox (Oracle)
> > > <willy@infradead.org> wrote:
> > > >
> > > > Encode the type into the bottom four bits of page->private and the
> > > > info into the remaining bits.  Also turn the bootmem type into a
> > > > named enum.
> >
> > > > @@ -21,9 +20,19 @@ enum {
> > > >  void __init register_page_bootmem_info_node(struct pglist_data *pgdat);
> > > >
> > > >  void get_page_bootmem(unsigned long info, struct page *page,
> > > > -                     unsigned long type);
> > > > +               enum bootmem_type type);
> > > >  void put_page_bootmem(struct page *page);
> > > >
> > > > +static inline enum bootmem_type bootmem_type(const struct page *page)
> > > > +{
> > > > +       return (unsigned long)page->private & 0xf;
> > > > +}
> > > > +
> > > > +static inline unsigned long bootmem_info(const struct page *page)
> > > > +{
> > > > +       return (unsigned long)page->private >> 4;
> > > > +}
> > >
> > > Would it be better to define the number of bits used for the type as a
> > > macro and use it throughout (bootmem_type(), bootmem_info(),
> > > get_page_bootmem())?.
> >
> > I think this is an adequate abstraction already.
> 
> It's not a big deal, but if we want to change the number of bits later
> we change one macro definition instead of 5 lines of code across two
> different files.

That doesn't sound like an obvious win to me.  Look, if you care, send
a patch.  What I have is definitely an improvement over what is there
now; you're just complaining that you think it could be even better,
and I disagree that what you want will be better than what I have.
The only reasonable approach is to merge my patch and then discuss
yours separately.