[Xen-devel] [PATCH] mm, frontswap: convert frontswap_enabled to static key
I have noticed that frontswap.h first declares "frontswap_enabled" as extern
bool variable, and then overrides it with "#define frontswap_enabled (1)" for
CONFIG_FRONTSWAP=Y or (0) when disabled. The bool variable isn't actually
instantiated anywhere.
This all looks like an unfinished attempt to make frontswap_enabled reflect
whether a backend is instantiated. But in the current state, all frontswap
hooks call unconditionally into frontswap.c just to check if frontswap_ops is
non-NULL. This should at least be checked inline, but we can further eliminate
the overhead when CONFIG_FRONTSWAP is enabled and no backend registered, using
a static key that is initially disabled, and gets enabled only upon first
backend registration.
Thus, checks for "frontswap_enabled" are replaced with "frontswap_enabled()"
wrapping the static key check. There are two exceptions:
- xen's selfballoon_process() was testing frontswap_enabled in code guarded
by #ifdef CONFIG_FRONTSWAP, which was effectively always true when reachable.
The patch just removes this check. Using frontswap_enabled() does not sound
correct here, as this can be true even without xen's own backend being
registered.
- in SYSCALL_DEFINE2(swapon), change the check to IS_ENABLED(CONFIG_FRONTSWAP)
as it seems the bitmap allocation cannot currently be postponed until a
backend is registered. This means that frontswap will still have some
memory overhead by being configured, but without a backend.
After the patch, we can expect that some functions in frontswap.c are called
only when frontswap_ops is non-NULL. Change the checks there to VM_BUG_ONs.
While at it, convert other BUG_ONs to VM_BUG_ONs as frontswap has been stable
for some time.
Signed-off-by: Vlastimil Babka <vba...@suse.cz>
---
drivers/xen/xen-selfballoon.c | 4 ++--
include/linux/frontswap.h | 34 --
mm/frontswap.c| 35 +++
mm/swapfile.c | 2 +-
4 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c
index 53a085fca00c..66620713242a 100644
--- a/drivers/xen/xen-selfballoon.c
+++ b/drivers/xen/xen-selfballoon.c
@@ -195,7 +195,7 @@ static void selfballoon_process(struct work_struct *work)
MB2PAGES(selfballoon_reserved_mb);
#ifdef CONFIG_FRONTSWAP
/* allow space for frontswap pages to be repatriated */
- if (frontswap_selfshrinking && frontswap_enabled)
+ if (frontswap_selfshrinking)
goal_pages += frontswap_curr_pages();
#endif
if (cur_pages > goal_pages)
@@ -230,7 +230,7 @@ static void selfballoon_process(struct work_struct *work)
reset_timer = true;
}
#ifdef CONFIG_FRONTSWAP
- if (frontswap_selfshrinking && frontswap_enabled) {
+ if (frontswap_selfshrinking) {
frontswap_selfshrink();
reset_timer = true;
}
diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index e65ef959546c..c46d2aa16d81 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
struct frontswap_ops {
void (*init)(unsigned); /* this swap type was just swapon'ed */
@@ -14,7 +15,6 @@ struct frontswap_ops {
struct frontswap_ops *next; /* private pointer to next ops */
};
-extern bool frontswap_enabled;
extern void frontswap_register_ops(struct frontswap_ops *ops);
extern void frontswap_shrink(unsigned long);
extern unsigned long frontswap_curr_pages(void);
@@ -30,7 +30,12 @@ extern void __frontswap_invalidate_page(unsigned, pgoff_t);
extern void __frontswap_invalidate_area(unsigned);
#ifdef CONFIG_FRONTSWAP
-#define frontswap_enabled (1)
+extern struct static_key_false frontswap_enabled_key;
+
+static inline bool frontswap_enabled(void)
+{
+ return static_branch_unlikely(_enabled_key);
+}
static inline bool frontswap_test(struct swap_info_struct *sis, pgoff_t offset)
{
@@ -50,7 +55,10 @@ static inline unsigned long *frontswap_map_get(struct
swap_info_struct *p)
#else
/* all inline routines become no-ops and all externs are ignored */
-#define frontswap_enabled (0)
+static inline bool frontswap_enabled(void)
+{
+ return false;
+}
static inline bool frontswap_test(struct swap_info_struct *sis, pgoff_t offset)
{
@@ -70,37 +78,35 @@ static inline unsigned long *frontswap_map_get(struct
swap_info_struct *p)
static inline int frontswap_store(struct page *page)
{
- int ret = -1;
+ if (frontswap_enabled())
+ return __frontswap_store(page);
- if (frontswap_enabled)
- ret = __frontswap_store(page);
- return ret;
+ return -1;
}
static inline int frontswap_load(struct page *page)
{
- int
Re: [Xen-devel] [PATCH] mm: Fix missing #include in
On 12/19/2015 09:30 PM, Ben Hutchings wrote: The various VM_WARN_ON/VM_BUG_ON macros depend on those defined by . Most users already include those, but not all; for example: CC arch/arm64/xen/../../arm/xen/grant-table.o In file included from arch/arm64/include/../../arm/include/asm/xen/page.h:5:0, from arch/arm64/include/asm/xen/page.h:1, from include/xen/page.h:28, from arch/arm64/xen/../../arm/xen/grant-table.c:33: arch/arm64/include/asm/pgtable.h: In function 'set_pte_at': arch/arm64/include/asm/pgtable.h:281:3: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration] VM_WARN_ONCE(!pte_young(pte), Signed-off-by: Ben Hutchings <b...@decadent.org.uk> Acked-by: Vlastimil Babka <vba...@suse.cz> --- include/linux/mmdebug.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h index 877ef22..772362a 100644 --- a/include/linux/mmdebug.h +++ b/include/linux/mmdebug.h @@ -1,6 +1,7 @@ #ifndef LINUX_MM_DEBUG_H #define LINUX_MM_DEBUG_H 1 +#include #include struct page; ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch V6 12/16] mm: provide early_memremap_ro to establish read-only mapping
On 08/06/2015 03:02 PM, Juergen Gross wrote: On 08/06/2015 02:46 PM, Vlastimil Babka wrote: On 07/17/2015 06:51 AM, Juergen Gross wrote: ... and here for !CONFIG_MMU. So, what about CONFIG_MMU !FIXMAP_PAGE_RO combinations? Which translates to CONFIG_MMU !PAGE_KERNEL_RO. Maybe they don't exist, but then it's still awkward to see the combination in the code left unimplemented. At least there are some architectures without #define PAGE_KERNEL_RO but testing CONFIG_MMU (arm, m68k, xtensa). Would it be perhaps simpler to assume the same thing as in drivers/base/firmware_class.c ? /* Some architectures don't have PAGE_KERNEL_RO */ #ifndef PAGE_KERNEL_RO #define PAGE_KERNEL_RO PAGE_KERNEL #endif Or would it be dangerous here to silently lose the read-only protection? The only reason to use this function instead of early_memremap() is the mandatory read-only mapping. My intention was to let the build fail in case it is being used but not implemented. An architecture requiring the function but having no PAGE_KERNEL_RO still can define FIXMAP_PAGE_RO. OK, in that case Acked-by: Vlastimil Babka vba...@suse.cz Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch V6 12/16] mm: provide early_memremap_ro to establish read-only mapping
On 07/17/2015 06:51 AM, Juergen Gross wrote:
During early boot as Xen pv domain the kernel needs to map some page
tables supplied by the hypervisor read only. This is needed to be
able to relocate some data structures conflicting with the physical
memory map especially on systems with huge RAM (above 512GB).
Provide the function early_memremap_ro() to provide this read only
mapping.
Signed-off-by: Juergen Gross jgr...@suse.com
Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Arnd Bergmann a...@arndb.de
Cc: linux...@kvack.org
Cc: linux-a...@vger.kernel.org
---
include/asm-generic/early_ioremap.h | 2 ++
include/asm-generic/fixmap.h| 3 +++
mm/early_ioremap.c | 12
3 files changed, 17 insertions(+)
diff --git a/include/asm-generic/early_ioremap.h
b/include/asm-generic/early_ioremap.h
index a5de55c..316bd04 100644
--- a/include/asm-generic/early_ioremap.h
+++ b/include/asm-generic/early_ioremap.h
@@ -11,6 +11,8 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
unsigned long size);
extern void *early_memremap(resource_size_t phys_addr,
unsigned long size);
+extern void *early_memremap_ro(resource_size_t phys_addr,
+ unsigned long size);
So the function is declared unconditionally...
extern void early_iounmap(void __iomem *addr, unsigned long size);
extern void early_memunmap(void *addr, unsigned long size);
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
index f23174f..1cbb833 100644
--- a/include/asm-generic/fixmap.h
+++ b/include/asm-generic/fixmap.h
@@ -46,6 +46,9 @@ static inline unsigned long virt_to_fix(const unsigned long
vaddr)
#ifndef FIXMAP_PAGE_NORMAL
#define FIXMAP_PAGE_NORMAL PAGE_KERNEL
#endif
+#if !defined(FIXMAP_PAGE_RO) defined(PAGE_KERNEL_RO)
+#define FIXMAP_PAGE_RO PAGE_KERNEL_RO
+#endif
#ifndef FIXMAP_PAGE_NOCACHE
#define FIXMAP_PAGE_NOCACHE PAGE_KERNEL_NOCACHE
#endif
diff --git a/mm/early_ioremap.c b/mm/early_ioremap.c
index e10ccd2..0cfadaf 100644
--- a/mm/early_ioremap.c
+++ b/mm/early_ioremap.c
@@ -217,6 +217,13 @@ early_memremap(resource_size_t phys_addr, unsigned long
size)
return (__force void *)__early_ioremap(phys_addr, size,
FIXMAP_PAGE_NORMAL);
}
+#ifdef FIXMAP_PAGE_RO
+void __init *
+early_memremap_ro(resource_size_t phys_addr, unsigned long size)
+{
+ return (__force void *)__early_ioremap(phys_addr, size, FIXMAP_PAGE_RO);
+}
+#endif
... here we provide a implementation when both CONFIG_MMU and
FIXMAP_PAGE_RO are defined...
#else /* CONFIG_MMU */
void __init __iomem *
@@ -231,6 +238,11 @@ early_memremap(resource_size_t phys_addr, unsigned long
size)
{
return (void *)phys_addr;
}
+void __init *
+early_memremap_ro(resource_size_t phys_addr, unsigned long size)
+{
+ return (void *)phys_addr;
+}
... and here for !CONFIG_MMU.
So, what about CONFIG_MMU !FIXMAP_PAGE_RO combinations? Which
translates to CONFIG_MMU !PAGE_KERNEL_RO. Maybe they don't exist, but
then it's still awkward to see the combination in the code left
unimplemented.
Would it be perhaps simpler to assume the same thing as in
drivers/base/firmware_class.c ?
/* Some architectures don't have PAGE_KERNEL_RO */
#ifndef PAGE_KERNEL_RO
#define PAGE_KERNEL_RO PAGE_KERNEL
#endif
Or would it be dangerous here to silently lose the read-only protection?
void __init early_iounmap(void __iomem *addr, unsigned long size)
{
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
