From: Thomas Gleixner <tglx@linutronix.de>
To: "André Almeida" <andrealmeid@igalia.com>,
"Ingo Molnar" <mingo@redhat.com>,
"Peter Zijlstra" <peterz@infradead.org>,
"Darren Hart" <dvhart@infradead.org>,
"Davidlohr Bueso" <dave@stgolabs.net>,
"Shuah Khan" <shuah@kernel.org>, "Arnd Bergmann" <arnd@arndb.de>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
"Waiman Long" <longman@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-api@vger.kernel.org, kernel-dev@igalia.com,
"André Almeida" <andrealmeid@igalia.com>
Subject: Re: [PATCH v5 7/7] selftests: futex: Expand robust list test for the new interface
Date: Fri, 27 Jun 2025 14:48:54 +0200 [thread overview]
Message-ID: <87tt41nydl.ffs@tglx> (raw)
In-Reply-To: <20250626-tonyk-robust_futex-v5-7-179194dbde8f@igalia.com>
On Thu, Jun 26 2025 at 14:11, André Almeida wrote:
> Expand the current robust list test for the new set_robust_list2
> syscall. Create an option to make it possible to run the same tests
> using the new syscall, and also add two new relevant test: test long
> lists (bigger than ROBUST_LIST_LIMIT) and for unaligned addresses.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> .../selftests/futex/functional/robust_list.c | 160 ++++++++++++++++++++-
> 1 file changed, 156 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/futex/functional/robust_list.c b/tools/testing/selftests/futex/functional/robust_list.c
> index 42690b2440fd29a9b12c46f67f9645ccc93d1147..004ad79ff6171c411fd47e699e3c38889544218e 100644
> --- a/tools/testing/selftests/futex/functional/robust_list.c
> +++ b/tools/testing/selftests/futex/functional/robust_list.c
> @@ -35,16 +35,45 @@
> #include <stddef.h>
> #include <sys/mman.h>
> #include <sys/wait.h>
> +#include <stdint.h>
>
> #define STACK_SIZE (1024 * 1024)
>
> #define FUTEX_TIMEOUT 3
>
> +#define SYS_set_robust_list2 468
> +
> +enum robust_list2_type {
> + ROBUST_LIST_32BIT,
> + ROBUST_LIST_64BIT,
> +};
Why can't this use an updated header?
> +
> static pthread_barrier_t barrier, barrier2;
>
> +bool robust2 = false;
global because ....
> int set_robust_list(struct robust_list_head *head, size_t len)
> {
> - return syscall(SYS_set_robust_list, head, len);
> + int ret, flags;
> +
> + if (!robust2) {
> + return syscall(SYS_set_robust_list, head, len);
> + }
Pointless brackets.
> + if (sizeof(head) == 8)
> + flags = ROBUST_LIST_64BIT;
> + else
> + flags = ROBUST_LIST_32BIT;
> +
> + /*
> + * We act as we have just one list here. We try to use the first slot,
> + * but if it hasn't been alocated yet we allocate it.
> + */
> + ret = syscall(SYS_set_robust_list2, head, 0, flags);
> + if (ret == -1 && errno == ENOENT)
> + ret = syscall(SYS_set_robust_list2, head, -1, flags);
What the heck is this?
> + return ret;
> }
>
> int get_robust_list(int pid, struct robust_list_head **head, size_t *len_ptr)
> @@ -246,6 +275,11 @@ static void test_set_robust_list_invalid_size(void)
> size_t head_size = sizeof(struct robust_list_head);
> int ret;
>
> + if (robust2) {
> + ksft_test_result_skip("This test is only for old robust interface\n");
Why is it invoked in the first place?
> + return;
> + }
> +
> ret = set_robust_list(&head, head_size);
> ASSERT_EQ(ret, 0);
>
> @@ -321,6 +355,11 @@ static void test_get_robust_list_child(void)
> struct robust_list_head head, *get_head;
> size_t len_ptr;
>
> + if (robust2) {
> + ksft_test_result_skip("Not implemented in the new robust interface\n");
For the very wrong reasons.
> + return;
> + }
> +
> ret = pthread_barrier_init(&barrier, NULL, 2);
> ret = pthread_barrier_init(&barrier2, NULL, 2);
> ASSERT_EQ(ret, 0);
> @@ -332,7 +371,7 @@ static void test_get_robust_list_child(void)
>
> ret = get_robust_list(tid, &get_head, &len_ptr);
> ASSERT_EQ(ret, 0);
> - ASSERT_EQ(&head, get_head);
> + ASSERT_EQ(get_head, &head);
ROTFL
>
> pthread_barrier_wait(&barrier2);
>
> @@ -507,11 +546,119 @@ static void test_circular_list(void)
> ksft_test_result_pass("%s\n", __func__);
> }
>
> +#define ROBUST_LIST_LIMIT 2048
> +#define CHILD_LIST_LIMIT (ROBUST_LIST_LIMIT + 10)
> +
> +static int child_robust_list_limit(void *arg)
> +{
> + struct lock_struct *locks;
> + struct robust_list *list;
> + struct robust_list_head head;
> + int ret, i;
> +
> + locks = (struct lock_struct *) arg;
> +
> + ret = set_list(&head);
> + if (ret)
> + ksft_test_result_fail("set_list error\n");
Yet again the same broken crap.
> + /*
> + * Create a very long list of locks
> + */
> + head.list.next = &locks[0].list;
> +
> + list = head.list.next;
> + for (i = 0; i < CHILD_LIST_LIMIT - 1; i++) {
> + list->next = &locks[i+1].list;
> + list = list->next;
> + }
> + list->next = &head.list;
> +
> + /*
> + * Grab the lock in the last one, and die without releasing it
> + */
> + mutex_lock(&locks[CHILD_LIST_LIMIT], &head, false);
> + pthread_barrier_wait(&barrier);
> +
> + sleep(1);
> +
> + return 0;
> +}
> +
> +/*
> + * The old robust list used to have a limit of 2048 items from the kernel side.
> + * After this limit the kernel stops walking the list and ignore the other
ignores
> + * futexes, causing deadlocks.
> + *
> + * For the new interface, test if we can wait for a list of more than 2048
> + * elements.
> + */
> +static void test_robust_list_limit(void)
> +{
> + struct lock_struct locks[CHILD_LIST_LIMIT + 1];
> + _Atomic(unsigned int) *futex = &locks[CHILD_LIST_LIMIT].futex;
> + struct robust_list_head head;
> + int ret;
> +
> + if (!robust2) {
> + ksft_test_result_skip("This test is only for new robust interface\n");
> + return;
> + }
> +
> + *futex = 0;
> +
> + ret = set_list(&head);
> + ASSERT_EQ(ret, 0);
> +
> + ret = pthread_barrier_init(&barrier, NULL, 2);
> + ASSERT_EQ(ret, 0);
> +
> + create_child(child_robust_list_limit, locks);
> +
> + /*
> + * After the child thread creates the very long list of locks, wait on
> + * the last one.
> + */
> + pthread_barrier_wait(&barrier);
> + ret = mutex_lock(&locks[CHILD_LIST_LIMIT], &head, false);
> +
> + if (ret != 0)
> + printf("futex wait returned %d\n", errno);
> + ASSERT_EQ(ret, 0);
lalala.
> +
> + ASSERT_TRUE(*futex | FUTEX_OWNER_DIED);
Copy and pasta does not make it more correct.
> + wait(NULL);
> + pthread_barrier_destroy(&barrier);
> +
> + ksft_test_result_pass("%s\n", __func__);
> +}
> +
> +/*
> + * The kernel should refuse an unaligned head pointer
> + */
> +static void test_unaligned_address(void)
> +{
> + struct robust_list_head head, *h;
> + int ret;
> +
> + if (!robust2) {
> + ksft_test_result_skip("This test is only for new robust interface\n");
> + return;
> + }
> +
> + h = (struct robust_list_head *) ((uintptr_t) &head + 1);
> + ret = set_list(h);
> + ASSERT_EQ(ret, -1);
> + ASSERT_EQ(errno, EINVAL);
> +}
> +
> void usage(char *prog)
> {
> printf("Usage: %s\n", prog);
> printf(" -c Use color\n");
> printf(" -h Display this help message\n");
> + printf(" -n Use robust2 syscall\n");
Right. We need a command line option to guarantee that the test is not
executed by bots...
> printf(" -v L Verbosity level: %d=QUIET %d=CRITICAL %d=INFO\n",
> VQUIET, VCRITICAL, VINFO);
> }
> @@ -520,7 +667,7 @@ int main(int argc, char *argv[])
> {
> int c;
>
> - while ((c = getopt(argc, argv, "cht:v:")) != -1) {
> + while ((c = getopt(argc, argv, "chnt:v:")) != -1) {
> switch (c) {
> case 'c':
> log_color(1);
> @@ -531,6 +678,9 @@ int main(int argc, char *argv[])
> case 'v':
> log_verbosity(atoi(optarg));
> break;
> + case 'n':
> + robust2 = true;
> + break;
> default:
> usage(basename(argv[0]));
> exit(1);
> @@ -538,7 +688,7 @@ int main(int argc, char *argv[])
> }
>
> ksft_print_header();
> - ksft_set_plan(7);
> + ksft_set_plan(8);
>
Just check whether the new syscall is implemented and then set the
number of tests accordingly.
> test_robustness();
>
> @@ -548,6 +698,8 @@ int main(int argc, char *argv[])
> test_set_list_op_pending();
> test_robust_list_multiple_elements();
> test_circular_list();
> + test_robust_list_limit();
> + test_unaligned_address();
and then do:
test_robustness();
....
test_circular_list();
if (has_robust) {
robust2 = true;
test_robustness();
...
test_circular_list();
test_robust_list_limit();
test_unaligned_address();
}
or something like that.
Time for a stiff drink....
prev parent reply other threads:[~2025-06-27 12:48 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-26 17:11 [PATCH v5 0/7] futex: Create set_robust_list2 André Almeida
2025-06-26 17:11 ` [PATCH v5 1/7] selftests/futex: Add ASSERT_ macros André Almeida
2025-06-26 22:07 ` Thomas Gleixner
2025-06-26 22:09 ` Thomas Gleixner
2025-06-27 20:23 ` André Almeida
2025-07-01 9:20 ` Thomas Gleixner
2025-06-26 17:11 ` [PATCH v5 2/7] selftests/futex: Create test for robust list André Almeida
2025-06-26 22:36 ` Thomas Gleixner
2025-06-26 17:11 ` [PATCH v5 3/7] futex: Use explicit sizes for compat_exit_robust_list André Almeida
2025-06-26 22:56 ` Thomas Gleixner
2025-06-28 14:27 ` kernel test robot
2025-06-26 17:11 ` [PATCH v5 4/7] futex: Create set_robust_list2 André Almeida
2025-06-27 12:06 ` Thomas Gleixner
2025-06-26 17:11 ` [PATCH v5 5/7] futex: Remove the limit of elements for sys_set_robust_list2 lists André Almeida
2025-06-27 12:22 ` Thomas Gleixner
2025-06-26 17:11 ` [PATCH v5 6/7] futex: Wire up set_robust_list2 syscall André Almeida
2025-06-26 17:11 ` [PATCH v5 7/7] selftests: futex: Expand robust list test for the new interface André Almeida
2025-06-27 12:48 ` Thomas Gleixner [this message]
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=87tt41nydl.ffs@tglx \
--to=tglx@linutronix.de \
--cc=andrealmeid@igalia.com \
--cc=arnd@arndb.de \
--cc=bigeasy@linutronix.de \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=kernel-dev@igalia.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=shuah@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.