On 10.03.21 18:49, Oscar Salvador wrote:
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.
Oh, I didn't feel offended - I was rather expressing that my opinion
might be biased because I came up with these names in the s390x variant ;)
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.
Thinking again, I guess it might be a good idea to factor out the core
functions into common code. For the optimization part, it might make
sense too pass some "state" structure that contains e.g.,
"unused_pmd_start".
Then we don't have diverging implementations of essentially the same thing.
Of course, we can do that on top of this series - unifying both
implementations.
--
Thanks,
David / dhildenb
