From: Robin Murphy <robin.murphy@arm.com>
To: linux-mm@kvack.org
Cc: dan.j.williams@intel.com, jglisse@redhat.com
Subject: __HAVE_ARCH_PTE_DEVMAP - bug or intended behaviour?
Date: Wed, 31 Oct 2018 17:08:23 +0000 [thread overview]
Message-ID: <9cf5c075-c83f-0915-99ef-b2aa59eca685@arm.com> (raw)
Hi mm folks,
I'm looking at ZONE_DEVICE support for arm64, and trying to make sense
of a build failure has led me down the rabbit hole of pfn_t.h, and
specifically __HAVE_ARCH_PTE_DEVMAP in this first instance.
The failure itself is a link error in remove_migration_pte() due to a
missing definition of pte_mkdevmap(), but I'm a little confused at the
fact that it's explicitly declared without a definition, as if that
breakage is deliberate.
So, is the !__HAVE_ARCH_PTE_DEVMAP case actually expected to work? If
not, then it seems to me that the relevant code could just be gated by
CONFIG_ZONE_DEVICE directly to remove the confusion. If it is, though,
then what should the generic definitions of p??_mkdevmap() be? I guess
either way I still need to figure out the implications of _PAGE_DEVMAP
at the arch end and whether/how arm64 should implement it, but given
this initial hurdle it's not clear exactly where to go next.
Tangentially, is it also right that is_device_{public,private}_page()
can still get non-stub definitions even with
CONFIG_DEVICE_{PUBLIC,PRIVATE} disabled? As it happens, the patch below
is enough to dodge the build failure for my configuration (i.e.
CONFIG_FS_DAX && !CONFIG_HMM) by optimising the offending call away,
however I'm not sure I'd want to rely on that; conceptually, though, it
does still seem like it might be appropriate.
Thanks,
Robin.
----->8-----
From: Robin Murphy <robin.murphy@arm.com>
Date: Wed, 31 Oct 2018 15:57:17 +0000
Subject: [PATCH] mm: Clean up is_device_*_page() definitions
Refactor is_device_{public,private}_page() with is_pci_p2pdma_page()
to make them all consistent in depending on their respective config
options even when CONFIG_DEV_PAGEMAP_OPS is enabled for other reasons.
This allows a little more compile-time optimisation as well as the
conceptual and cosmetic cleanup.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
include/linux/mm.h | 52 ++++++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1e52b8fd1685..15a49ed5436c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -879,32 +879,6 @@ static inline bool put_devmap_managed_page(struct
page *page)
}
return false;
}
-
-static inline bool is_device_private_page(const struct page *page)
-{
- return is_zone_device_page(page) &&
- page->pgmap->type == MEMORY_DEVICE_PRIVATE;
-}
-
-static inline bool is_device_public_page(const struct page *page)
-{
- return is_zone_device_page(page) &&
- page->pgmap->type == MEMORY_DEVICE_PUBLIC;
-}
-
-#ifdef CONFIG_PCI_P2PDMA
-static inline bool is_pci_p2pdma_page(const struct page *page)
-{
- return is_zone_device_page(page) &&
- page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
-}
-#else /* CONFIG_PCI_P2PDMA */
-static inline bool is_pci_p2pdma_page(const struct page *page)
-{
- return false;
-}
-#endif /* CONFIG_PCI_P2PDMA */
-
#else /* CONFIG_DEV_PAGEMAP_OPS */
static inline void dev_pagemap_get_ops(void)
{
@@ -918,22 +892,46 @@ static inline bool put_devmap_managed_page(struct
page *page)
{
return false;
}
+#endif /* CONFIG_DEV_PAGEMAP_OPS */
+#if defined(CONFIG_DEV_PAGEMAP_OPS) && defined(CONFIG_DEVICE_PRIVATE)
+static inline bool is_device_private_page(const struct page *page)
+{
+ return is_zone_device_page(page) &&
+ page->pgmap->type == MEMORY_DEVICE_PRIVATE;
+}
+#else
static inline bool is_device_private_page(const struct page *page)
{
return false;
}
+#endif
+#if defined(CONFIG_DEV_PAGEMAP_OPS) && defined(CONFIG_DEVICE_PUBLIC)
+static inline bool is_device_public_page(const struct page *page)
+{
+ return is_zone_device_page(page) &&
+ page->pgmap->type == MEMORY_DEVICE_PUBLIC;
+}
+#else
static inline bool is_device_public_page(const struct page *page)
{
return false;
}
+#endif
+#if defined(CONFIG_DEV_PAGEMAP_OPS) && defined(CONFIG_PCI_P2PDMA)
+static inline bool is_pci_p2pdma_page(const struct page *page)
+{
+ return is_zone_device_page(page) &&
+ page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
+}
+#else
static inline bool is_pci_p2pdma_page(const struct page *page)
{
return false;
}
-#endif /* CONFIG_DEV_PAGEMAP_OPS */
+#endif
static inline void get_page(struct page *page)
{
--
2.19.1.dirty
next reply other threads:[~2018-10-31 17:08 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-31 17:08 Robin Murphy [this message]
2018-10-31 19:00 ` __HAVE_ARCH_PTE_DEVMAP - bug or intended behaviour? Jerome Glisse
2018-10-31 20:35 ` Dan Williams
2018-10-31 20:41 ` Dan Williams
2018-11-01 20:10 ` Robin Murphy
2018-11-01 20:58 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9cf5c075-c83f-0915-99ef-b2aa59eca685@arm.com \
--to=robin.murphy@arm.com \
--cc=dan.j.williams@intel.com \
--cc=jglisse@redhat.com \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).