On 7/29/19 4:51 PM, Song Liu wrote:
+#define HPAGE_PMD_OFFSET (HPAGE_PMD_SIZE - 1)
^ space vs. tab difference here.
Thanks, good catch!
+#define HPAGE_PMD_MASK (~(HPAGE_PMD_OFFSET))
+
+#define HPAGE_PUD_SHIFT PUD_SHIFT
+#define HPAGE_PUD_SIZE ((1UL) << HPAGE_PUD_SHIFT)
+#define HPAGE_PUD_OFFSET (HPAGE_PUD_SIZE - 1)
Saw this one, too.
Should HPAGE_PMD_OFFSET and HPAGE_PUD_OFFSET include bits for
PAGE_OFFSET? I guess we can just keep huge_mm.h as-is and use
~HPAGE_PMD_MASK.
That's what I had intended; would you rather see those macros
omit the unneeded for the larger page size bits?
- * - FGP_PMD: If FGP_CREAT is specified, attempt to allocate a PMD-sized page.
+ * - FGP_PMD: If FGP_CREAT is specified, attempt to allocate a PMD-sized page
No - this came in as part of patch 1/2 and I missed dropping the period
at the end of the line that caused this to be a diff, so I will put it
back. :-)
We have been using name "xas" for "struct xa_state *". Let's keep using it?
Thanks, done.
+ if (unlikely(!(PageCompound(new_page)))) {
What condition triggers this case
I wanted a check to make sure that __page_cacke_alloc() returned a large
page. I don't recall if the mechanism guarantees that when you ask for
a large page, you get one, so I wanted to handle that case.
If you prefer, I could make this a VM_BUG_ON_PAGE() instead, but I
wanted it to fallback gracefully if it can't get a properly sized
page.
+#ifndef COMPOUND_PAGES_HEAD_ONLY
Where do we define COMPOUND_PAGES_HEAD_ONLY?
At present, we do not.
I used this so I could include the code that would be needed once
Matthew's "store only head pages in page cache" changes go back in,
which looks like it may not be until 5.4-rc1. Matthew recommended I
include this so we didn't lose track of the code change that would be
needed then. I'll be talking to him today about this and the issues
you raised regarding patch 1/2.
Thanks for going through this!!
-- Bill