Re: [PATCH v5 3/4] x86/vmemmap: Handle unpopulated sub-pmd ranges

From: Oscar Salvador
Date: Wed Mar 10 2021 - 12:50:47 EST


On Tue, Mar 09, 2021 at 06:52:38PM +0100, David Hildenbrand wrote:
> > -#define PAGE_INUSE 0xFD
> > +#ifdef CONFIG_SPARSEMEM_VMEMMAP
> > +#define PAGE_UNUSED 0xFD
> > +
> > +/* Returns true if the PMD is completely unused and thus it can be freed */
> > +static bool __meminit vmemmap_pmd_is_unused(unsigned long addr, unsigned long end)
> > +{
>
> I don't think the new name is any better. It implies that all it does is a
> check - yet it actually clears the given range. (I prefer the old name, but
> well, I came up with that, so what do I know :D )

Sorry, I did not mean to offend here.

Something like: vmemmap_is_pmd_unused_after_clearing_it would be a bit better
I guess.
Tbh, both this and previous one looked fine to me, but I understand where Dave
confusion was coming from, that is why I decided to rename it.

Maybe a middle-ground would have been to expand the comment above.

--
Oscar Salvador
SUSE L3