On Fri, Jan 27, 2023 at 11:47:14AM +0500, Muhammad Usama Anjum wrote:
> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index 4000e9f017e0..8c03b133d483 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c
> >> @@ -3351,6 +3351,18 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
> >>
> >> if (likely(!unshare)) {
> >> if (userfaultfd_pte_wp(vma, *vmf->pte)) {
> >> + if (userfaultfd_wp_async(vma)) {
> >> + /*
> >> + * Nothing needed (cache flush, TLB invalidations,
> >> + * etc.) because we're only removing the uffd-wp bit,
> >> + * which is completely invisible to the user. This
> >> + * falls through to possible CoW.
> >
> > Here it says it falls through to CoW, but..
> >
> >> + */
> >> + pte_unmap_unlock(vmf->pte, vmf->ptl);
> >> + set_pte_at(vma->vm_mm, vmf->address, vmf->pte,
> >> + pte_clear_uffd_wp(*vmf->pte));
> >> + return 0;
> >
> > ... it's not doing so. The original lines should do:
> >
> > https://lore.kernel.org/all/Y8qq0dKIJBshua+X@x1n/
[1]
> >
> > Side note: you cannot modify pgtable after releasing the pgtable lock.
> > It's racy.
> If I don't unlock and return after removing the UFFD_WP flag in case of
> async wp, the target just gets stuck. Maybe the pte lock is not unlocked in
> some path.
>
> If I unlock and don't return, the crash happens.
>
> So I'd put unlock and return from here. Please comment on the below patch
> and what do you think should be done. I've missed something.
Have you tried to just use exactly what I suggested in [1]? I'll paste
again:
---8<---
diff --git a/mm/memory.c b/mm/memory.c
index 4000e9f017e0..09aab434654c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3351,8 +3351,20 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
if (likely(!unshare)) {
if (userfaultfd_pte_wp(vma, *vmf->pte)) {
- pte_unmap_unlock(vmf->pte, vmf->ptl);
- return handle_userfault(vmf, VM_UFFD_WP);
+ if (userfaultfd_uffd_wp_async(vma)) {
+ /*
+ * Nothing needed (cache flush, TLB
+ * invalidations, etc.) because we're only
+ * removing the uffd-wp bit, which is
+ * completely invisible to the user.
+ * This falls through to possible CoW.
+ */
+ set_pte_at(vma->vm_mm, vmf->address, vmf->pte,
+ pte_clear_uffd_wp(*vmf->pte));
+ } else {
+ pte_unmap_unlock(vmf->pte, vmf->ptl);
+ return handle_userfault(vmf, VM_UFFD_WP);
+ }
}
---8<---
Note that there's no "return", neither the unlock. The lock is used in the
follow up write fault resolution and it's released later.
Meanwhile please fully digest how pgtable lock is used in this path before
moving forward on any of such changes.
>
> >
> >> + }
> >> pte_unmap_unlock(vmf->pte, vmf->ptl);
> >> return handle_userfault(vmf, VM_UFFD_WP);
> >> }
> >> @@ -4812,8 +4824,21 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf)
> >>
> >> if (vma_is_anonymous(vmf->vma)) {
> >> if (likely(!unshare) &&
> >> - userfaultfd_huge_pmd_wp(vmf->vma, vmf->orig_pmd))
> >> - return handle_userfault(vmf, VM_UFFD_WP);
> >> + userfaultfd_huge_pmd_wp(vmf->vma, vmf->orig_pmd)) {
> >> + if (userfaultfd_wp_async(vmf->vma)) {
> >> + /*
> >> + * Nothing needed (cache flush, TLB invalidations,
> >> + * etc.) because we're only removing the uffd-wp bit,
> >> + * which is completely invisible to the user. This
> >> + * falls through to possible CoW.
> >> + */
> >> + set_pmd_at(vmf->vma->vm_mm, vmf->address, vmf->pmd,
> >> + pmd_clear_uffd_wp(*vmf->pmd));
> >
> > This is for THP, not hugetlb.
> >
> > Clearing uffd-wp bit here for the whole pmd is wrong to me, because we
> > track writes in small page sizes only. We should just split.
> By detecting if the fault is async wp, just splitting the PMD doesn't work.
> The below given snippit is working right now. But definately, the fault of
> the whole PMD is being resolved which if we can bypass by correctly
> splitting would be highly desirable. Can you please take a look on UFFD
> side and suggest the changes? It would be much appreciated. I'm attaching
> WIP v9 patches for you to apply on next(next-20230105) and pagemap_ioctl
> selftest can be ran to test things after making changes.
Can you elaborate why thp split didn't work? Or if you want, I can look
into this and provide the patch to enable uffd async mode.
Thanks,
--
Peter Xu
Add a simple way of redirecting calls to functions by including a
special prologue in the "real" function which checks to see if the
replacement function should be called (and, if so, calls it).
To redirect calls to a function, make the first (non-declaration) line
of the function:
KUNIT_STATIC_STUB_REDIRECT(function_name, [function arguments]);
(This will compile away to nothing if KUnit is not enabled, otherwise it
will check if a redirection is active, call the replacement function,
and return. This check is protected by a static branch, so has very
little overhead when there are no KUnit tests running.)
Calls to the real function can be redirected to a replacement using:
kunit_activate_static_stub(test, real_fn, replacement_fn);
The redirection will only affect calls made from within the kthread of
the current test, and will be automatically disabled when the test
completes. It can also be manually disabled with
kunit_deactivate_static_stub().
The 'example' KUnit test suite has a more complete example.
Co-developed-by: Daniel Latypov <dlatypov(a)google.com>
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
Signed-off-by: David Gow <davidgow(a)google.com>
Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com>
---
This patch depends upon the 'hooks' implementation in
https://lore.kernel.org/linux-kselftest/20230128071007.1134942-1-davidgow@g…
Note that checkpatch.pl does warn about control flow in the
KUNIT_STATIC_STUB_REDIRECT() macro. This is an intentional design choice
(we think it makes the feature easier to use), though if there are
strong objections, we can of course reconsider.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20230128074918.1180523-1-davidgow@g…
- The example comment for KUNIT_STATIC_STUB_REDIRECT() now uses the
correct 'int' return type. (Thanks, Brendan)
Changes since v1:
https://lore.kernel.org/all/20221208061841.2186447-2-davidgow@google.com/
- Adapted to use the "hooks" mechanism
- See: https://lore.kernel.org/linux-kselftest/20230128071007.1134942-1-davidgow@g…
- Now works when KUnit itself is compiled as a module (CONFIG_KUNIT=m)
Changes since RFC v2:
https://lore.kernel.org/linux-kselftest/20220910212804.670622-2-davidgow@go…
- Now uses the kunit_get_current_test() function, which uses the static
key to reduce overhead.
- Thanks Kees for the suggestion.
- Note that this does prevent redirections from working when
CONFIG_KUNIT=m -- this is a restriction of kunit_get_current_test()
which will be removed in a future patch.
- Several tidy-ups to the inline documentation.
Changes since RFC v1:
https://lore.kernel.org/lkml/20220318021314.3225240-2-davidgow@google.com/
- Use typecheck_fn() to fix typechecking in some cases (thanks Brendan)
---
include/kunit/static_stub.h | 113 ++++++++++++++++++++++++++++++
include/kunit/test-bug.h | 1 +
lib/kunit/Makefile | 1 +
lib/kunit/hooks-impl.h | 2 +
lib/kunit/kunit-example-test.c | 38 ++++++++++
lib/kunit/static_stub.c | 123 +++++++++++++++++++++++++++++++++
6 files changed, 278 insertions(+)
create mode 100644 include/kunit/static_stub.h
create mode 100644 lib/kunit/static_stub.c
diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h
new file mode 100644
index 000000000000..9b80150a5d62
--- /dev/null
+++ b/include/kunit/static_stub.h
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit function redirection (static stubbing) API.
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: David Gow <davidgow(a)google.com>
+ */
+#ifndef _KUNIT_STATIC_STUB_H
+#define _KUNIT_STATIC_STUB_H
+
+#if !IS_ENABLED(CONFIG_KUNIT)
+
+/* If CONFIG_KUNIT is not enabled, these stubs quietly disappear. */
+#define KUNIT_TRIGGER_STATIC_STUB(real_fn_name, args...) do {} while (0)
+
+#else
+
+#include <kunit/test.h>
+#include <kunit/test-bug.h>
+
+#include <linux/compiler.h> /* for {un,}likely() */
+#include <linux/sched.h> /* for task_struct */
+
+
+/**
+ * KUNIT_STATIC_STUB_REDIRECT() - call a replacement 'static stub' if one exists
+ * @real_fn_name: The name of this function (as an identifier, not a string)
+ * @args: All of the arguments passed to this function
+ *
+ * This is a function prologue which is used to allow calls to the current
+ * function to be redirected by a KUnit test. KUnit tests can call
+ * kunit_activate_static_stub() to pass a replacement function in. The
+ * replacement function will be called by KUNIT_TRIGGER_STATIC_STUB(), which
+ * will then return from the function. If the caller is not in a KUnit context,
+ * the function will continue execution as normal.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * int real_func(int n)
+ * {
+ * KUNIT_STATIC_STUB_REDIRECT(real_func, n);
+ * return 0;
+ * }
+ *
+ * int replacement_func(int n)
+ * {
+ * return 42;
+ * }
+ *
+ * void example_test(struct kunit *test)
+ * {
+ * kunit_activate_static_stub(test, real_func, replacement_func);
+ * KUNIT_EXPECT_EQ(test, real_func(1), 42);
+ * }
+ *
+ */
+#define KUNIT_STATIC_STUB_REDIRECT(real_fn_name, args...) \
+do { \
+ typeof(&real_fn_name) replacement; \
+ struct kunit *current_test = kunit_get_current_test(); \
+ \
+ if (likely(!current_test)) \
+ break; \
+ \
+ replacement = kunit_hooks.get_static_stub_address(current_test, \
+ &real_fn_name); \
+ \
+ if (unlikely(replacement)) \
+ return replacement(args); \
+} while (0)
+
+/* Helper function for kunit_activate_static_stub(). The macro does
+ * typechecking, so use it instead.
+ */
+void __kunit_activate_static_stub(struct kunit *test,
+ void *real_fn_addr,
+ void *replacement_addr);
+
+/**
+ * kunit_activate_static_stub() - replace a function using static stubs.
+ * @test: A pointer to the 'struct kunit' test context for the current test.
+ * @real_fn_addr: The address of the function to replace.
+ * @replacement_addr: The address of the function to replace it with.
+ *
+ * When activated, calls to real_fn_addr from within this test (even if called
+ * indirectly) will instead call replacement_addr. The function pointed to by
+ * real_fn_addr must begin with the static stub prologue in
+ * KUNIT_TRIGGER_STATIC_STUB() for this to work. real_fn_addr and
+ * replacement_addr must have the same type.
+ *
+ * The redirection can be disabled again with kunit_deactivate_static_stub().
+ */
+#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do { \
+ typecheck_fn(typeof(&real_fn_addr), replacement_addr); \
+ __kunit_activate_static_stub(test, real_fn_addr, replacement_addr); \
+} while (0)
+
+
+/**
+ * kunit_deactivate_static_stub() - disable a function redirection
+ * @test: A pointer to the 'struct kunit' test context for the current test.
+ * @real_fn_addr: The address of the function to no-longer redirect
+ *
+ * Deactivates a redirection configured with kunit_activate_static_stub(). After
+ * this function returns, calls to real_fn_addr() will execute the original
+ * real_fn, not any previously-configured replacement.
+ */
+void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr);
+
+#endif
+#endif
diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
index 2b505a95b641..30ca541b6ff2 100644
--- a/include/kunit/test-bug.h
+++ b/include/kunit/test-bug.h
@@ -20,6 +20,7 @@ DECLARE_STATIC_KEY_FALSE(kunit_running);
/* Hooks table: a table of function pointers filled in when kunit loads */
extern struct kunit_hooks_table {
__printf(3, 4) void (*fail_current_test)(const char*, int, const char*, ...);
+ void *(*get_static_stub_address)(struct kunit *test, void *real_fn_addr);
} kunit_hooks;
/**
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index deeb46cc879b..da665cd4ea12 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_KUNIT) += kunit.o
kunit-objs += test.o \
resource.o \
+ static_stub.o \
string-stream.o \
assert.o \
try-catch.o \
diff --git a/lib/kunit/hooks-impl.h b/lib/kunit/hooks-impl.h
index d911f40f76db..ec745a39832c 100644
--- a/lib/kunit/hooks-impl.h
+++ b/lib/kunit/hooks-impl.h
@@ -16,12 +16,14 @@
/* List of declarations. */
void __kunit_fail_current_test_impl(const char *file, int line, const char *fmt, ...);
+void *__kunit_get_static_stub_address_impl(struct kunit *test, void *real_fn_addr);
/* Code to set all of the function pointers. */
static inline void kunit_install_hooks(void)
{
/* Install the KUnit hook functions. */
kunit_hooks.fail_current_test = __kunit_fail_current_test_impl;
+ kunit_hooks.get_static_stub_address = __kunit_get_static_stub_address_impl;
}
#endif /* _KUNIT_HOOKS_IMPL_H */
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 66cc4e2365ec..cd8b7e51d02b 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -7,6 +7,7 @@
*/
#include <kunit/test.h>
+#include <kunit/static_stub.h>
/*
* This is the most fundamental element of KUnit, the test case. A test case
@@ -130,6 +131,42 @@ static void example_all_expect_macros_test(struct kunit *test)
KUNIT_ASSERT_GT_MSG(test, sizeof(int), 0, "Your ints are 0-bit?!");
}
+/* This is a function we'll replace with static stubs. */
+static int add_one(int i)
+{
+ /* This will trigger the stub if active. */
+ KUNIT_STATIC_STUB_REDIRECT(add_one, i);
+
+ return i + 1;
+}
+
+/* This is used as a replacement for the above function. */
+static int subtract_one(int i)
+{
+ /* We don't need to trigger the stub from the replacement. */
+
+ return i - 1;
+}
+
+/*
+ * This test shows the use of static stubs.
+ */
+static void example_static_stub_test(struct kunit *test)
+{
+ /* By default, function is not stubbed. */
+ KUNIT_EXPECT_EQ(test, add_one(1), 2);
+
+ /* Replace add_one() with subtract_one(). */
+ kunit_activate_static_stub(test, add_one, subtract_one);
+
+ /* add_one() is now replaced. */
+ KUNIT_EXPECT_EQ(test, add_one(1), 0);
+
+ /* Return add_one() to normal. */
+ kunit_deactivate_static_stub(test, add_one);
+ KUNIT_EXPECT_EQ(test, add_one(1), 2);
+}
+
/*
* Here we make a list of all the test cases we want to add to the test suite
* below.
@@ -145,6 +182,7 @@ static struct kunit_case example_test_cases[] = {
KUNIT_CASE(example_skip_test),
KUNIT_CASE(example_mark_skipped_test),
KUNIT_CASE(example_all_expect_macros_test),
+ KUNIT_CASE(example_static_stub_test),
{}
};
diff --git a/lib/kunit/static_stub.c b/lib/kunit/static_stub.c
new file mode 100644
index 000000000000..92b2cccd5e76
--- /dev/null
+++ b/lib/kunit/static_stub.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit function redirection (static stubbing) API.
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: David Gow <davidgow(a)google.com>
+ */
+
+#include <kunit/test.h>
+#include <kunit/static_stub.h>
+#include "hooks-impl.h"
+
+
+/* Context for a static stub. This is stored in the resource data. */
+struct kunit_static_stub_ctx {
+ void *real_fn_addr;
+ void *replacement_addr;
+};
+
+static void __kunit_static_stub_resource_free(struct kunit_resource *res)
+{
+ kfree(res->data);
+}
+
+/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
+static bool __kunit_static_stub_resource_match(struct kunit *test,
+ struct kunit_resource *res,
+ void *match_real_fn_addr)
+{
+ /* This pointer is only valid if res is a static stub resource. */
+ struct kunit_static_stub_ctx *ctx = res->data;
+
+ /* Make sure the resource is a static stub resource. */
+ if (res->free != &__kunit_static_stub_resource_free)
+ return false;
+
+ return ctx->real_fn_addr == match_real_fn_addr;
+}
+
+/* Hook to return the address of the replacement function. */
+void *__kunit_get_static_stub_address_impl(struct kunit *test, void *real_fn_addr)
+{
+ struct kunit_resource *res;
+ struct kunit_static_stub_ctx *ctx;
+ void *replacement_addr;
+
+ res = kunit_find_resource(test,
+ __kunit_static_stub_resource_match,
+ real_fn_addr);
+
+ if (!res)
+ return NULL;
+
+ ctx = res->data;
+ replacement_addr = ctx->replacement_addr;
+ kunit_put_resource(res);
+ return replacement_addr;
+}
+
+void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr)
+{
+ struct kunit_resource *res;
+
+ KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+ "Tried to deactivate a NULL stub.");
+
+ /* Look up the existing stub for this function. */
+ res = kunit_find_resource(test,
+ __kunit_static_stub_resource_match,
+ real_fn_addr);
+
+ /* Error out if the stub doesn't exist. */
+ KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
+ "Tried to deactivate a nonexistent stub.");
+
+ /* Free the stub. We 'put' twice, as we got a reference
+ * from kunit_find_resource()
+ */
+ kunit_remove_resource(test, res);
+ kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_deactivate_static_stub);
+
+/* Helper function for kunit_activate_static_stub(). The macro does
+ * typechecking, so use it instead.
+ */
+void __kunit_activate_static_stub(struct kunit *test,
+ void *real_fn_addr,
+ void *replacement_addr)
+{
+ struct kunit_static_stub_ctx *ctx;
+ struct kunit_resource *res;
+
+ KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+ "Tried to activate a stub for function NULL");
+
+ /* If the replacement address is NULL, deactivate the stub. */
+ if (!replacement_addr) {
+ kunit_deactivate_static_stub(test, replacement_addr);
+ return;
+ }
+
+ /* Look up any existing stubs for this function, and replace them. */
+ res = kunit_find_resource(test,
+ __kunit_static_stub_resource_match,
+ real_fn_addr);
+ if (res) {
+ ctx = res->data;
+ ctx->replacement_addr = replacement_addr;
+
+ /* We got an extra reference from find_resource(), so put it. */
+ kunit_put_resource(res);
+ } else {
+ ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+ ctx->real_fn_addr = real_fn_addr;
+ ctx->replacement_addr = replacement_addr;
+ res = kunit_alloc_resource(test, NULL,
+ &__kunit_static_stub_resource_free,
+ GFP_KERNEL, ctx);
+ }
+}
+EXPORT_SYMBOL_GPL(__kunit_activate_static_stub);
--
2.39.1.456.gfc5497dd1b-goog
KUnit has several macros and functions intended for use from non-test
code. These hooks, currently the kunit_get_current_test() and
kunit_fail_current_test() macros, didn't work when CONFIG_KUNIT=m.
In order to support this case, the required functions and static data
need to be available unconditionally, even when KUnit itself is not
built-in. The new 'hooks.c' file is therefore always included, and has
both the static key required for kunit_get_current_test(), and a table
of function pointers in struct kunit_hooks_table. This is filled in with
the real implementations by kunit_install_hooks(), which is kept in
hooks-impl.h and called when the kunit module is loaded.
This can be extended for future features which require similar
"hook" behaviour, such as static stubs, by simply adding new entries to
the struct, and the appropriate code to set them.
Signed-off-by: David Gow <davidgow(a)google.com>
---
This is basically a prerequisite for the stub features working when
KUnit is built as a module, and should nicely make a few other tests
work, too.
This version uses a struct, rather than a bunch of separate function
pointers, to define the list of hooks in one place. It also doesn't use
the macro magic from RFC v2 (which we could reintroduce later if we end
up with enough hooks that it'd make sense). It does get rid of all of
the nasty checkpatch.pl warnings, though, save for:
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#230: FILE: lib/kunit/hooks.c:16:
+EXPORT_SYMBOL(kunit_running);
This is a false-positive, as the EXPORT_SYMBOL() immediately follows the
DEFINE_STATIC_KEY_FALSE() macro, which checkpatch doesn't recognise as a
definition.
Cheers,
-- David
Changes since RFC v2:
https://lore.kernel.org/linux-kselftest/20230124080350.2275652-1-davidgow@g…
- Get rid of the macro magic, and keep the function pointers in a
struct.
- Also, reset them to NULL using memset, so we don't need to loop
through all of them manually.
- Thanks Daniel!
- Properly forward-declare all of the implementations, now in
"hooks-impl.h", so they can easily be split across different files.
(Needed for the stubs implementation.)
- Extract the stub installation into a separate function,
kunit_install_hooks().
- Thanks Daniel!
Changes since RFC v1:
https://lore.kernel.org/all/20230117142737.246446-1-davidgow@google.com/
- Major refit to auto-generate the hook code using macros.
- (Note that previous Reviewed-by tags have not been added, as this is a
big enough change it probably needs a re-reviews. Thanks Rae for
reviewing RFC v1 previously, though!)
---
Documentation/dev-tools/kunit/usage.rst | 14 ++++++-------
include/kunit/test-bug.h | 28 +++++++++----------------
lib/Makefile | 8 +++++++
lib/kunit/Makefile | 3 +++
lib/kunit/hooks-impl.h | 27 ++++++++++++++++++++++++
lib/kunit/hooks.c | 21 +++++++++++++++++++
lib/kunit/test.c | 14 ++++++-------
7 files changed, 82 insertions(+), 33 deletions(-)
create mode 100644 lib/kunit/hooks-impl.h
create mode 100644 lib/kunit/hooks.c
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 48f8196d5aad..6424493b93cb 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -648,10 +648,9 @@ We can do this via the ``kunit_test`` field in ``task_struct``, which we can
access using the ``kunit_get_current_test()`` function in ``kunit/test-bug.h``.
``kunit_get_current_test()`` is safe to call even if KUnit is not enabled. If
-KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is
-running in the current task, it will return ``NULL``. This compiles down to
-either a no-op or a static key check, so will have a negligible performance
-impact when no test is running.
+KUnit is not enabled, or if no test is running in the current task, it will
+return ``NULL``. This compiles down to either a no-op or a static key check,
+so will have a negligible performance impact when no test is running.
The example below uses this to implement a "mock" implementation of a function, ``foo``:
@@ -726,8 +725,7 @@ structures as shown below:
#endif
``kunit_fail_current_test()`` is safe to call even if KUnit is not enabled. If
-KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is
-running in the current task, it will do nothing. This compiles down to either a
-no-op or a static key check, so will have a negligible performance impact when
-no test is running.
+KUnit is not enabled, or if no test is running in the current task, it will do
+nothing. This compiles down to either a no-op or a static key check, so will
+have a negligible performance impact when no test is running.
diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
index c1b2e14eab64..2b505a95b641 100644
--- a/include/kunit/test-bug.h
+++ b/include/kunit/test-bug.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * KUnit API allowing dynamic analysis tools to interact with KUnit tests
+ * KUnit API providing hooks for non-test code to interact with tests.
*
* Copyright (C) 2020, Google LLC.
* Author: Uriel Guajardo <urielguajardo(a)google.com>
@@ -9,7 +9,7 @@
#ifndef _KUNIT_TEST_BUG_H
#define _KUNIT_TEST_BUG_H
-#if IS_BUILTIN(CONFIG_KUNIT)
+#if IS_ENABLED(CONFIG_KUNIT)
#include <linux/jump_label.h> /* For static branch */
#include <linux/sched.h>
@@ -17,6 +17,11 @@
/* Static key if KUnit is running any tests. */
DECLARE_STATIC_KEY_FALSE(kunit_running);
+/* Hooks table: a table of function pointers filled in when kunit loads */
+extern struct kunit_hooks_table {
+ __printf(3, 4) void (*fail_current_test)(const char*, int, const char*, ...);
+} kunit_hooks;
+
/**
* kunit_get_current_test() - Return a pointer to the currently running
* KUnit test.
@@ -43,33 +48,20 @@ static inline struct kunit *kunit_get_current_test(void)
* kunit_fail_current_test() - If a KUnit test is running, fail it.
*
* If a KUnit test is running in the current task, mark that test as failed.
- *
- * This macro will only work if KUnit is built-in (though the tests
- * themselves can be modules). Otherwise, it compiles down to nothing.
*/
#define kunit_fail_current_test(fmt, ...) do { \
if (static_branch_unlikely(&kunit_running)) { \
- __kunit_fail_current_test(__FILE__, __LINE__, \
+ /* Guaranteed to be non-NULL when kunit_running true*/ \
+ kunit_hooks.fail_current_test(__FILE__, __LINE__, \
fmt, ##__VA_ARGS__); \
} \
} while (0)
-
-extern __printf(3, 4) void __kunit_fail_current_test(const char *file, int line,
- const char *fmt, ...);
-
#else
static inline struct kunit *kunit_get_current_test(void) { return NULL; }
-/* We define this with an empty helper function so format string warnings work */
-#define kunit_fail_current_test(fmt, ...) \
- __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__)
-
-static inline __printf(3, 4) void __kunit_fail_current_test(const char *file, int line,
- const char *fmt, ...)
-{
-}
+#define kunit_fail_current_test(fmt, ...) do {} while (0)
#endif
diff --git a/lib/Makefile b/lib/Makefile
index 4d9461bfea42..55fd04a7d0fb 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -126,6 +126,14 @@ CFLAGS_test_fpu.o += $(FPU_CFLAGS)
obj-$(CONFIG_TEST_LIVEPATCH) += livepatch/
obj-$(CONFIG_KUNIT) += kunit/
+# Include the KUnit hooks unconditionally. They'll compile to nothing if
+# CONFIG_KUNIT=n, otherwise will be a small table of static data (static key,
+# function pointers) which need to be built-in even when KUnit is a module.
+ifeq ($(CONFIG_KUNIT), m)
+obj-y += kunit/hooks.o
+else
+obj-$(CONFIG_KUNIT) += kunit/hooks.o
+endif
ifeq ($(CONFIG_DEBUG_KOBJECT),y)
CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 29aff6562b42..deeb46cc879b 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -11,6 +11,9 @@ ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
kunit-objs += debugfs.o
endif
+# KUnit 'hooks' are built-in even when KUnit is built as a module.
+lib-y += hooks.o
+
obj-$(CONFIG_KUNIT_TEST) += kunit-test.o
# string-stream-test compiles built-in only.
diff --git a/lib/kunit/hooks-impl.h b/lib/kunit/hooks-impl.h
new file mode 100644
index 000000000000..d911f40f76db
--- /dev/null
+++ b/lib/kunit/hooks-impl.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Declarations for hook implementations.
+ *
+ * These will be set as the function pointers in struct kunit_hook_table,
+ * found in include/kunit/test-bug.h.
+ *
+ * Copyright (C) 2023, Google LLC.
+ * Author: David Gow <davidgow(a)google.com>
+ */
+
+#ifndef _KUNIT_HOOKS_IMPL_H
+#define _KUNIT_HOOKS_IMPL_H
+
+#include <kunit/test-bug.h>
+
+/* List of declarations. */
+void __kunit_fail_current_test_impl(const char *file, int line, const char *fmt, ...);
+
+/* Code to set all of the function pointers. */
+static inline void kunit_install_hooks(void)
+{
+ /* Install the KUnit hook functions. */
+ kunit_hooks.fail_current_test = __kunit_fail_current_test_impl;
+}
+
+#endif /* _KUNIT_HOOKS_IMPL_H */
diff --git a/lib/kunit/hooks.c b/lib/kunit/hooks.c
new file mode 100644
index 000000000000..365d98d4953c
--- /dev/null
+++ b/lib/kunit/hooks.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit 'Hooks' implementation.
+ *
+ * This file contains code / structures which should be built-in even when
+ * KUnit itself is built as a module.
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: David Gow <davidgow(a)google.com>
+ */
+
+
+#include <kunit/test-bug.h>
+
+DEFINE_STATIC_KEY_FALSE(kunit_running);
+EXPORT_SYMBOL(kunit_running);
+
+/* Function pointers for hooks. */
+struct kunit_hooks_table kunit_hooks;
+EXPORT_SYMBOL(kunit_hooks);
+
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c9ebf975e56b..51cae59d8aae 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -17,16 +17,14 @@
#include <linux/sched.h>
#include "debugfs.h"
+#include "hooks-impl.h"
#include "string-stream.h"
#include "try-catch-impl.h"
-DEFINE_STATIC_KEY_FALSE(kunit_running);
-
-#if IS_BUILTIN(CONFIG_KUNIT)
/*
- * Fail the current test and print an error message to the log.
+ * Hook to fail the current test and print an error message to the log.
*/
-void __kunit_fail_current_test(const char *file, int line, const char *fmt, ...)
+void __kunit_fail_current_test_impl(const char *file, int line, const char *fmt, ...)
{
va_list args;
int len;
@@ -53,8 +51,6 @@ void __kunit_fail_current_test(const char *file, int line, const char *fmt, ...)
kunit_err(current->kunit_test, "%s:%d: %s", file, line, buffer);
kunit_kfree(current->kunit_test, buffer);
}
-EXPORT_SYMBOL_GPL(__kunit_fail_current_test);
-#endif
/*
* Enable KUnit tests to run.
@@ -777,6 +773,9 @@ EXPORT_SYMBOL_GPL(kunit_cleanup);
static int __init kunit_init(void)
{
+ /* Install the KUnit hook functions. */
+ kunit_install_hooks();
+
kunit_debugfs_init();
#ifdef CONFIG_MODULES
return register_module_notifier(&kunit_mod_nb);
@@ -788,6 +787,7 @@ late_initcall(kunit_init);
static void __exit kunit_exit(void)
{
+ memset(&kunit_hooks, 0, sizeof(kunit_hooks));
#ifdef CONFIG_MODULES
unregister_module_notifier(&kunit_mod_nb);
#endif
--
2.39.1.456.gfc5497dd1b-goog
Real-time setups try hard to ensure proper isolation between time
critical applications and e.g. network processing performed by the
network stack in softirq and RPS is used to move the softirq
activity away from the isolated core.
If the network configuration is dynamic, with netns and devices
routinely created at run-time, enforcing the correct RPS setting
on each newly created device allowing to transient bad configuration
became complex.
These series try to address the above, introducing a new
sysctl knob: rps_default_mask. The new sysctl entry allows
configuring a systemwide RPS mask, to be enforced since receive
queue creation time without any fourther per device configuration
required.
Additionally, a simple self-test is introduced to check the
rps_default_mask behavior.
v1 -> v2:
- fix sparse warning in patch 2/3
Paolo Abeni (3):
net/sysctl: factor-out netdev_rx_queue_set_rps_mask() helper
net/core: introduce default_rps_mask netns attribute
self-tests: introduce self-tests for RPS default mask
Documentation/admin-guide/sysctl/net.rst | 6 ++
include/linux/netdevice.h | 1 +
net/core/net-sysfs.c | 73 +++++++++++--------
net/core/sysctl_net_core.c | 58 +++++++++++++++
tools/testing/selftests/net/Makefile | 1 +
tools/testing/selftests/net/config | 3 +
.../testing/selftests/net/rps_default_mask.sh | 57 +++++++++++++++
7 files changed, 169 insertions(+), 30 deletions(-)
create mode 100755 tools/testing/selftests/net/rps_default_mask.sh
--
2.26.2
*Changes in v8:*
- Update uffd async wp implementation
- Improve PAGEMAP_IOCTL implementation
*Changes in v7:*
- Add uffd wp async
- Update the IOCTL to use uffd under the hood instead of soft-dirty
flags
Hello,
Note:
Soft-dirty pages and pages which have been written-to are synonyms. As
kernel already has soft-dirty feature inside which we have given up to
use, we are using written-to terminology while using UFFD async WP under
the hood.
This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear
the info about page table entries. The following operations are
supported in this ioctl:
- Get the information if the pages have been written-to (PAGE_IS_WT),
file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped
(PAGE_IS_SWAPPED).
- Write-protect the pages (PAGEMAP_WP_ENGAGE) to start finding which
pages have been written-to.
- Find pages which have been written-to and write protect the pages
(atomic PAGE_IS_WT + PAGEMAP_WP_ENGAGE)
It is possible to find and clear soft-dirty pages entirely in userspace.
But it isn't efficient:
- The mprotect and SIGSEGV handler for bookkeeping
- The userfaultfd wp with the handler for bookkeeping
Some benchmarks can be seen here[1]. This series adds features that weren't
present earlier:
- There is no atomic get soft-dirty PTE bit status and clear present in
the kernel.
- The pages which have been written-to can not be found in accurate way.
(Kernel's soft-dirty PTE bit + sof_dirty VMA bit shows more soft-dirty
pages than there actually are.)
Historically, soft-dirty PTE bit tracking has been used in the CRIU
project. The procfs interface is enough for finding the soft-dirty bit
status and clearing the soft-dirty bit of all the pages of a process.
We have the use case where we need to track the soft-dirty PTE bit for
only specific pages on demand. We need this tracking and clear mechanism
of a region of memory while the process is running to emulate the
getWriteWatch() syscall of Windows.
*(Moved to using UFFD instead of soft-dirty to find pages which have been
written-to from v7 patch series)*:
Stop using the soft-dirty flags for finding which pages have been
written to. It is too delicate and wrong as it shows more soft-dirty
pages than the actual soft-dirty pages. There is no interest in
correcting it [2][3] as this is how the feature was written years ago.
It shouldn't be updated to changed behaviour. Peter Xu has suggested
using the async version of the UFFD WP [4] as it is based inherently
on the PTEs.
So in this patch series, I've added a new mode to the UFFD which is
asynchronous version of the write protect. When this variant of the
UFFD WP is used, the page faults are resolved automatically by the
kernel. The pages which have been written-to can be found by reading
pagemap file (!PM_UFFD_WP). This feature can be used successfully to
find which pages have been written to from the time the pages were
write protected. This works just like the soft-dirty flag without
showing any extra pages which aren't soft-dirty in reality.
The information related to pages if the page is file mapped, present and
swapped is required for the CRIU project [5][6]. The addition of the
required mask, any mask, excluded mask and return masks are also required
for the CRIU project [5].
The IOCTL returns the addresses of the pages which match the specific masks.
The page addresses are returned in struct page_region in a compact form.
The max_pages is needed to support a use case where user only wants to get
a specific number of pages. So there is no need to find all the pages of
interest in the range when max_pages is specified. The IOCTL returns when
the maximum number of the pages are found. The max_pages is optional. If
max_pages is specified, it must be equal or greater than the vec_size.
This restriction is needed to handle worse case when one page_region only
contains info of one page and it cannot be compacted. This is needed to
emulate the Windows getWriteWatch() syscall.
The patch series include the detailed selftest which can be used as an example
for the uffd async wp test and PAGEMAP_IOCTL. It shows the interface usages as
well.
[1] https://lore.kernel.org/lkml/54d4c322-cd6e-eefd-b161-2af2b56aae24@collabora…
[2] https://lore.kernel.org/all/20221220162606.1595355-1-usama.anjum@collabora.…
[3] https://lore.kernel.org/all/20221122115007.2787017-1-usama.anjum@collabora.…
[4] https://lore.kernel.org/all/Y6Hc2d+7eTKs7AiH@x1n
[5] https://lore.kernel.org/all/YyiDg79flhWoMDZB@gmail.com/
[6] https://lore.kernel.org/all/20221014134802.1361436-1-mdanylo@google.com/
Regards,
Muhammad Usama Anjum
Muhammad Usama Anjum (4):
userfaultfd: Add UFFD WP Async support
userfaultfd: split mwriteprotect_range()
fs/proc/task_mmu: Implement IOCTL to get and/or the clear info about
PTEs
selftests: vm: add pagemap ioctl tests
fs/proc/task_mmu.c | 294 +++++++
fs/userfaultfd.c | 21 +
include/linux/userfaultfd_k.h | 16 +
include/uapi/linux/fs.h | 50 ++
include/uapi/linux/userfaultfd.h | 8 +-
mm/memory.c | 29 +-
mm/userfaultfd.c | 40 +-
tools/include/uapi/linux/fs.h | 50 ++
tools/testing/selftests/vm/.gitignore | 1 +
tools/testing/selftests/vm/Makefile | 5 +-
tools/testing/selftests/vm/pagemap_ioctl.c | 880 +++++++++++++++++++++
11 files changed, 1374 insertions(+), 20 deletions(-)
create mode 100644 tools/testing/selftests/vm/pagemap_ioctl.c
--
2.30.2