[LTP] [PATCH v2] syscalls/set_thread_area01: Refactor into new API
Andrea Cervesato
andrea.cervesato@suse.com
Thu Apr 3 15:22:45 CEST 2025
Hi Ricardo,
On 4/3/25 14:36, Ricardo B. Marlière via ltp wrote:
> From: Ricardo B. Marlière <rbm@suse.com>
>
> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
> ---
> Changes in v2:
> - Used tst_test.test_all instead of .test
> - Removed VALUE_AND_STRING macro
> - Added local wrappers to the syscalls
> - Fixed the test description
> - Added a comment quote from the set_thread_area manual to add context
> - Link to v1: https://lore.kernel.org/all/20250327-conversions-modify_ldt-v2-6-2907d4d3f6c0@suse.com
> ---
> .../syscalls/set_thread_area/set_thread_area.h | 31 -----
> .../syscalls/set_thread_area/set_thread_area01.c | 133 +++++++--------------
> 2 files changed, 43 insertions(+), 121 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/set_thread_area/set_thread_area.h b/testcases/kernel/syscalls/set_thread_area/set_thread_area.h
> deleted file mode 100644
> index 2bd2469d549ecf8c5c589a0a9485f886d043f7ed..0000000000000000000000000000000000000000
> --- a/testcases/kernel/syscalls/set_thread_area/set_thread_area.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -#include <stdio.h>
> -#include <errno.h>
> -
> -/* Harness Specific Include Files. */
> -#include "test.h"
> -#include "lapi/syscalls.h"
> -#include "config.h"
> -
> -#if defined HAVE_ASM_LDT_H
> -#include <linux/unistd.h>
> -#include <asm/ldt.h>
> -
> -#if defined HAVE_STRUCT_USER_DESC
> -typedef struct user_desc thread_area_s;
> -#elif defined HAVE_STRUCT_MODIFY_LDT_LDT_S
> -typedef struct modify_ldt_ldt_s thread_area_s;
> -#else
> -typedef struct user_desc {
> - unsigned int entry_number;
> - unsigned long int base_addr;
> - unsigned int limit;
> - unsigned int seg_32bit:1;
> - unsigned int contents:2;
> - unsigned int read_exec_only:1;
> - unsigned int limit_in_pages:1;
> - unsigned int seg_not_present:1;
> - unsigned int useable:1;
> - unsigned int empty:25;
> -} thread_area_s;
> -#endif /* HAVE_STRUCT_USER_DESC */
> -#endif /* HAVE_ASM_LDT_H */
> diff --git a/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c b/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
> index 30626d5e90ebf8e20624c370cc4474cb51a6b102..ba8d90c826c668fd94edecb95d121ec7c6bbaa06 100644
> --- a/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
> +++ b/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
> @@ -1,111 +1,64 @@
> -/*************************************************************************
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> * Copyright (c) Crackerjack Project., 2007
> * Copyright (c) Manas Kumar Nayak <maknayak@in.ibm.com>
> * Copyright (c) Cyril Hrubis <chrubis@suse.cz> 2011
> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
> + */
> +
> +/*\
> + * Basic test of i386 thread-local storage for set_thread_area and
> + * get_thread_area syscalls.
This is not only a basic test, but also a test that verifies errors for
set_thread_area and get_thread_area syscalls.
> *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> - * the GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * [Algorithm]
> *
> - ************************************************************************/
> + * The test will first call set_thread_area to a struct user_desc pointer with
> + * entry_number = -1, which will be set to a free entry_number upon exiting.
> + * Therefore, a subsequent call to get_thread_area using the same pointer will
> + * be valid. The same process is done but using -2 instead, which returns EINVAL
> + * to both calls. Finally, it's done one more time but to an invalid pointer and
> + * therefore an EFAULT is returned.
It's usually better to describe the algorithm using dotted list, so it's
easier to read.
> + */
>
> -#include "set_thread_area.h"
> +#include "tst_test.h"
>
> -char *TCID = "set_thread_area_01";
> -int TST_TOTAL = 6;
> +#ifdef __i386__
> +#include "lapi/ldt.h"
>
> -#if defined(HAVE_ASM_LDT_H) && defined(HAVE_STRUCT_USER_DESC)
> +/* When set_thread_area() is passed an entry_number of -1, it searches
> + * for a free TLS entry. If set_thread_area() finds a free TLS entry,
> + * the value of u_info->entry_number is set upon return to show which
> + * entry was changed.
> + */
> +static struct user_desc entry = { .entry_number = -1 };
> +static struct user_desc invalid_entry = { .entry_number = -2 };
>
> -static void cleanup(void)
> +static int set_thread_area(const struct user_desc *entry)
> {
> + return tst_syscall(__NR_set_thread_area, entry);
> }
>
> -static void setup(void)
> +static int get_thread_area(const struct user_desc *entry)
> {
> - TEST_PAUSE;
> + return tst_syscall(__NR_get_thread_area, entry);
> }
>
> -struct test {
> - int syscall;
> - const char *const syscall_name;
> - thread_area_s *u_info;
> - int exp_ret;
> - int exp_errno;
> -};
> -
> -/*
> - * The set_thread_area uses a free entry_number if entry number is set to -1
> - * and upon the syscall exit the entry number is set to entry which was used.
> - * So when we call get_thread_area on u_info1, the entry number is initalized
> - * corectly by the previous set_thread_area.
> - */
> -static struct user_desc u_info1 = {.entry_number = -1 };
> -static struct user_desc u_info2 = {.entry_number = -2 };
> -
> -#define VALUE_AND_STRING(val) val, #val
> -
> -static struct test tests[] = {
> - {VALUE_AND_STRING(__NR_set_thread_area), &u_info1, 0, 0},
> - {VALUE_AND_STRING(__NR_get_thread_area), &u_info1, 0, 0},
> - {VALUE_AND_STRING(__NR_set_thread_area), &u_info2, -1, EINVAL},
> - {VALUE_AND_STRING(__NR_get_thread_area), &u_info2, -1, EINVAL},
> - {VALUE_AND_STRING(__NR_set_thread_area), (void *)-9, -1, EFAULT},
> - {VALUE_AND_STRING(__NR_get_thread_area), (void *)-9, -1, EFAULT},
> -};
This was a good idea, but the implementation doesn't fit with the LTP
API. Test cases in our scenario might improve readability. In
particular, we can do something like this:
- define 2 variant for the test (check tst_variant). The first variant
will call set_thread_area and the second will call get_thread_area
- define 2 test cases. The first will set input value to generate a
EINVAL and the secon will set input value to generate EFAULT
And this is the first test that will check for errors only: it will run
both error tests for set_thread_area and get_thread_area. The second
test is only calling set+get and verify that we have TPASS from syscalls
and data set is equivalent to data read.
In this way we can organize the testing suite properly and split errors
from base functionalities as we do in the other testing suites. WDYT?
> -
> -int main(int argc, char *argv[])
> +void run(void)
> {
> - int lc;
> - unsigned i;
> -
> - tst_parse_opts(argc, argv, NULL, NULL);
> -
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> - for (i = 0; i < sizeof(tests) / sizeof(struct test); i++) {
> - TEST(tst_syscall(tests[i].syscall, tests[i].u_info));
> + TST_EXP_PASS(set_thread_area(&entry));
> + TST_EXP_PASS(get_thread_area(&entry));
>
> - if (TEST_RETURN != tests[i].exp_ret) {
> - tst_resm(TFAIL, "%s returned %li expected %i",
> - tests[i].syscall_name,
> - TEST_RETURN, tests[i].exp_ret);
> - continue;
> - }
> + TST_EXP_FAIL(set_thread_area(&invalid_entry), EINVAL);
> + TST_EXP_FAIL(get_thread_area(&invalid_entry), EINVAL);
>
> - if (TEST_ERRNO != tests[i].exp_errno) {
> - tst_resm(TFAIL,
> - "%s failed with %i (%s) expected %i (%s)",
> - tests[i].syscall_name, TEST_ERRNO,
> - strerror(TEST_ERRNO),
> - tests[i].exp_errno,
> - strerror(tests[i].exp_errno));
> - continue;
> - }
> + TST_EXP_FAIL(set_thread_area((void *)-9), EFAULT);
> + TST_EXP_FAIL(get_thread_area((void *)-9), EFAULT);
> +}
>
> - tst_resm(TPASS, "%s returned %li errno %i (%s)",
> - tests[i].syscall_name, TEST_RETURN,
> - TEST_ERRNO, strerror(TEST_ERRNO));
> - }
> - }
> +static struct tst_test test = {
> + .test_all = run,
> +};
>
> - cleanup();
> - tst_exit();
> -}
> #else
> -int main(void)
> -{
> - tst_brkm(TCONF, NULL,
> - "set_thread_area isn't available for this architecture");
> -}
> -#endif
> +TST_TEST_TCONF("Test supported only on i386");
> +#endif /* __i386__ */
>
> ---
> base-commit: 898cc14ad412fb521867b43fed5c4e067b76f809
This is a super cool feature from b4 :-)
> change-id: 20250403-conversions-set_thread_area-07a90e0cd449
> prerequisite-message-id: <20250402-conversions-modify_ldt-v6-0-2e4b0e27870e@suse.com>
> prerequisite-patch-id: 490a3e6bc4004db5234224b6fd6d4bf5030b219d
> prerequisite-patch-id: 962bb815444eb2de9756dd2659e097567b6d6fe8
> prerequisite-patch-id: a8357fb870a8d7278e206b4fc65f1d9450fee802
>
> Best regards,
Kind regards,
Andrea Cervesato
More information about the ltp
mailing list