* [LTP] [PATCH 1/3] mbind01: make use of tst_numa_mode_name
@ 2021-07-29 13:25 Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 2/3] libs: rename tst_numa_mode_name to tst_mempolicy_mode_name Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL Li Wang
0 siblings, 2 replies; 10+ messages in thread
From: Li Wang @ 2021-07-29 13:25 UTC (permalink / raw)
To: ltp
Add MPOL_LOCAL and adjust some mempolicy mode modes order.
To prettify the error log of mbind01.
Signed-off-by: Li Wang <liwang@redhat.com>
---
libs/libltpnuma/tst_numa.c | 7 +++++--
testcases/kernel/syscalls/mbind/mbind01.c | 10 ++++++----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
index 56c8640ff..d2241eeae 100644
--- a/libs/libltpnuma/tst_numa.c
+++ b/libs/libltpnuma/tst_numa.c
@@ -16,6 +16,7 @@
#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"
#include "tst_numa.h"
+#include "lapi/numaif.h"
void tst_nodemap_print_counters(struct tst_nodemap *nodes)
{
@@ -50,12 +51,14 @@ const char *tst_numa_mode_name(int mode)
switch (mode) {
case MPOL_DEFAULT:
return "MPOL_DEFAULT";
- case MPOL_BIND:
- return "MPOL_BIND";
case MPOL_PREFERRED:
return "MPOL_PREFERRED";
+ case MPOL_BIND:
+ return "MPOL_BIND";
case MPOL_INTERLEAVE:
return "MPOL_INTERLEAVE";
+ case MPOL_LOCAL:
+ return "MPOL_LOCAL";
default:
return "???";
}
diff --git a/testcases/kernel/syscalls/mbind/mbind01.c b/testcases/kernel/syscalls/mbind/mbind01.c
index de46c9381..5a2f37307 100644
--- a/testcases/kernel/syscalls/mbind/mbind01.c
+++ b/testcases/kernel/syscalls/mbind/mbind01.c
@@ -17,6 +17,7 @@
#include "config.h"
#include "numa_helper.h"
#include "tst_test.h"
+#include "tst_numa.h"
#include "lapi/numaif.h"
#ifdef HAVE_NUMA_V2
@@ -124,9 +125,9 @@ static struct test_case tcase[] = {
static void check_policy_pref_no_target(int policy)
{
if (policy != MPOL_PREFERRED && policy != MPOL_LOCAL) {
- tst_res(TFAIL, "Wrong policy: %d, "
+ tst_res(TFAIL, "Wrong policy: %s(%d), "
"expected MPOL_PREFERRED or MPOL_LOCAL",
- policy);
+ tst_numa_mode_name(policy), policy);
}
}
@@ -200,8 +201,9 @@ static void do_test(unsigned int i)
if (tc->check_policy)
tc->check_policy(policy);
else if (tc->policy != policy) {
- tst_res(TFAIL, "Wrong policy: %d, expected: %d",
- policy, tc->policy);
+ tst_res(TFAIL, "Wrong policy: %s(%d), expected: %s(%d)",
+ tst_numa_mode_name(policy), policy,
+ tst_numa_mode_name(tc->policy), tc->policy);
fail = 1;
}
if (tc->exp_nodemask) {
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [LTP] [PATCH 2/3] libs: rename tst_numa_mode_name to tst_mempolicy_mode_name
2021-07-29 13:25 [LTP] [PATCH 1/3] mbind01: make use of tst_numa_mode_name Li Wang
@ 2021-07-29 13:25 ` Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL Li Wang
1 sibling, 0 replies; 10+ messages in thread
From: Li Wang @ 2021-07-29 13:25 UTC (permalink / raw)
To: ltp
To be precise describe the MPOL_* mempolicy mode and be consistent
with kernel's consuetude.
Signed-off-by: Li Wang <liwang@redhat.com>
---
include/tst_numa.h | 2 +-
libs/libltpnuma/tst_numa.c | 2 +-
testcases/kernel/syscalls/mbind/mbind01.c | 6 +++---
testcases/kernel/syscalls/mbind/mbind02.c | 6 +++---
testcases/kernel/syscalls/mbind/mbind03.c | 4 ++--
testcases/kernel/syscalls/mbind/mbind04.c | 4 ++--
testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c | 4 ++--
testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c | 4 ++--
8 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/tst_numa.h b/include/tst_numa.h
index 846e093a9..3af311e5e 100644
--- a/include/tst_numa.h
+++ b/include/tst_numa.h
@@ -38,7 +38,7 @@ void tst_nodemap_print_counters(struct tst_nodemap *nodes);
*
* @mode Numa mempolicy mode.
*/
-const char *tst_numa_mode_name(int mode);
+const char *tst_mempolicy_mode_name(int mode);
/**
* Maps pages into memory, if path is NULL the mapping is anonymous otherwise is backed by the file.
diff --git a/libs/libltpnuma/tst_numa.c b/libs/libltpnuma/tst_numa.c
index d2241eeae..e3e80f229 100644
--- a/libs/libltpnuma/tst_numa.c
+++ b/libs/libltpnuma/tst_numa.c
@@ -46,7 +46,7 @@ void tst_nodemap_free(struct tst_nodemap *nodes)
#ifdef HAVE_NUMA_V2
-const char *tst_numa_mode_name(int mode)
+const char *tst_mempolicy_mode_name(int mode)
{
switch (mode) {
case MPOL_DEFAULT:
diff --git a/testcases/kernel/syscalls/mbind/mbind01.c b/testcases/kernel/syscalls/mbind/mbind01.c
index 5a2f37307..d2cf13c8f 100644
--- a/testcases/kernel/syscalls/mbind/mbind01.c
+++ b/testcases/kernel/syscalls/mbind/mbind01.c
@@ -127,7 +127,7 @@ static void check_policy_pref_no_target(int policy)
if (policy != MPOL_PREFERRED && policy != MPOL_LOCAL) {
tst_res(TFAIL, "Wrong policy: %s(%d), "
"expected MPOL_PREFERRED or MPOL_LOCAL",
- tst_numa_mode_name(policy), policy);
+ tst_mempolicy_mode_name(policy), policy);
}
}
@@ -202,8 +202,8 @@ static void do_test(unsigned int i)
tc->check_policy(policy);
else if (tc->policy != policy) {
tst_res(TFAIL, "Wrong policy: %s(%d), expected: %s(%d)",
- tst_numa_mode_name(policy), policy,
- tst_numa_mode_name(tc->policy), tc->policy);
+ tst_mempolicy_mode_name(policy), policy,
+ tst_mempolicy_mode_name(tc->policy), tc->policy);
fail = 1;
}
if (tc->exp_nodemask) {
diff --git a/testcases/kernel/syscalls/mbind/mbind02.c b/testcases/kernel/syscalls/mbind/mbind02.c
index 01b3b3c69..4b0851e57 100644
--- a/testcases/kernel/syscalls/mbind/mbind02.c
+++ b/testcases/kernel/syscalls/mbind/mbind02.c
@@ -74,18 +74,18 @@ static void verify_policy(int mode)
if (TST_RET != -1) {
tst_res(TFAIL,
"mbind(%s, MPOL_MF_STRICT) node %u returned %li, expected -1",
- tst_numa_mode_name(mode), node, TST_RET);
+ tst_mempolicy_mode_name(mode), node, TST_RET);
return;
}
if (TST_ERR == EIO) {
tst_res(TPASS | TTERRNO,
"mbind(%s, MPOL_MF_STRICT) node %u",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
} else {
tst_res(TFAIL | TTERRNO,
"mbind(%s, MPOL_MF_STRICT) node %u expected EIO",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
}
}
diff --git a/testcases/kernel/syscalls/mbind/mbind03.c b/testcases/kernel/syscalls/mbind/mbind03.c
index c0750c804..392f89787 100644
--- a/testcases/kernel/syscalls/mbind/mbind03.c
+++ b/testcases/kernel/syscalls/mbind/mbind03.c
@@ -68,11 +68,11 @@ static void verify_policy(int mode, unsigned flag)
if (TST_RET) {
tst_res(TFAIL | TTERRNO,
"mbind(%s, %s) node %u",
- tst_numa_mode_name(mode), mbind_flag_name(flag), node);
+ tst_mempolicy_mode_name(mode), mbind_flag_name(flag), node);
goto exit;
} else {
tst_res(TPASS, "mbind(%s, %s) node %u succeded",
- tst_numa_mode_name(mode), mbind_flag_name(flag), node);
+ tst_mempolicy_mode_name(mode), mbind_flag_name(flag), node);
}
tst_nodemap_reset_counters(nodes);
diff --git a/testcases/kernel/syscalls/mbind/mbind04.c b/testcases/kernel/syscalls/mbind/mbind04.c
index ea9966622..e7603691a 100644
--- a/testcases/kernel/syscalls/mbind/mbind04.c
+++ b/testcases/kernel/syscalls/mbind/mbind04.c
@@ -61,12 +61,12 @@ static void verify_policy(unsigned int node, int mode, unsigned flag)
if (TST_RET) {
tst_res(TFAIL | TTERRNO,
"mbind(%s, %s) node %u",
- tst_numa_mode_name(mode), mbind_flag_name(flag), node);
+ tst_mempolicy_mode_name(mode), mbind_flag_name(flag), node);
return;
}
tst_res(TPASS, "mbind(%s, %s) node %u",
- tst_numa_mode_name(mode), mbind_flag_name(flag), node);
+ tst_mempolicy_mode_name(mode), mbind_flag_name(flag), node);
const char *prefix = "child: ";
diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c
index 96a275411..3f36fbe62 100644
--- a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c
+++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c
@@ -55,12 +55,12 @@ static void verify_mempolicy(unsigned int node, int mode)
if (TST_RET) {
tst_res(TFAIL | TTERRNO,
"set_mempolicy(%s) node %u",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
return;
}
tst_res(TPASS, "set_mempolicy(%s) node %u",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
numa_free_nodemask(bm);
diff --git a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c
index 24775de10..2fb43e98d 100644
--- a/testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c
+++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c
@@ -54,12 +54,12 @@ static void verify_mempolicy(unsigned int node, int mode)
if (TST_RET) {
tst_res(TFAIL | TTERRNO,
"set_mempolicy(%s) node %u",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
return;
}
tst_res(TPASS, "set_mempolicy(%s) node %u",
- tst_numa_mode_name(mode), node);
+ tst_mempolicy_mode_name(mode), node);
numa_free_nodemask(bm);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-29 13:25 [LTP] [PATCH 1/3] mbind01: make use of tst_numa_mode_name Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 2/3] libs: rename tst_numa_mode_name to tst_mempolicy_mode_name Li Wang
@ 2021-07-29 13:25 ` Li Wang
2021-07-29 14:20 ` Jan Stancek
1 sibling, 1 reply; 10+ messages in thread
From: Li Wang @ 2021-07-29 13:25 UTC (permalink / raw)
To: ltp
Signed-off-by: Li Wang <liwang@redhat.com>
---
testcases/kernel/syscalls/mbind/mbind01.c | 25 ++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/testcases/kernel/syscalls/mbind/mbind01.c b/testcases/kernel/syscalls/mbind/mbind01.c
index d2cf13c8f..b5c1e948d 100644
--- a/testcases/kernel/syscalls/mbind/mbind01.c
+++ b/testcases/kernel/syscalls/mbind/mbind01.c
@@ -34,7 +34,7 @@ static struct bitmask *nodemask, *getnodemask, *empty_nodemask;
static void test_default(unsigned int i, char *p);
static void test_none(unsigned int i, char *p);
static void test_invalid_nodemask(unsigned int i, char *p);
-static void check_policy_pref_no_target(int);
+static void check_policy_pref_or_local(int);
struct test_case {
int policy;
@@ -92,7 +92,7 @@ static struct test_case tcase[] = {
.ret = 0,
.err = 0,
.test = test_none,
- .check_policy = check_policy_pref_no_target,
+ .check_policy = check_policy_pref_or_local,
},
{
POLICY_DESC(MPOL_PREFERRED),
@@ -101,6 +101,20 @@ static struct test_case tcase[] = {
.test = test_default,
.exp_nodemask = &nodemask,
},
+ {
+ POLICY_DESC(MPOL_LOCAL),
+ .ret = 0,
+ .err = 0,
+ .test = test_none,
+ .exp_nodemask = &empty_nodemask,
+ .check_policy = check_policy_pref_or_local,
+ },
+ {
+ POLICY_DESC_TEXT(MPOL_LOCAL, "target exists"),
+ .ret = -1,
+ .err = EINVAL,
+ .test = test_default,
+ },
{
POLICY_DESC(UNKNOWN_POLICY),
.ret = -1,
@@ -122,7 +136,7 @@ static struct test_case tcase[] = {
},
};
-static void check_policy_pref_no_target(int policy)
+static void check_policy_pref_or_local(int policy)
{
if (policy != MPOL_PREFERRED && policy != MPOL_LOCAL) {
tst_res(TFAIL, "Wrong policy: %s(%d), "
@@ -182,6 +196,11 @@ static void do_test(unsigned int i)
tst_res(TINFO, "case %s", tc->desc);
+ if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
+ tst_res(TCONF, "%s is not supported", tst_mempolicy_mode_name(tc->policy));
+ return;
+ }
+
setup_node();
p = SAFE_MMAP(NULL, MEM_LENGTH, PROT_READ | PROT_WRITE, MAP_PRIVATE |
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-29 13:25 ` [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL Li Wang
@ 2021-07-29 14:20 ` Jan Stancek
2021-07-30 8:03 ` Li Wang
0 siblings, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2021-07-29 14:20 UTC (permalink / raw)
To: ltp
On Thu, Jul 29, 2021 at 3:25 PM Li Wang <liwang@redhat.com> wrote:
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
> testcases/kernel/syscalls/mbind/mbind01.c | 25 ++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mbind/mbind01.c
> b/testcases/kernel/syscalls/mbind/mbind01.c
> index d2cf13c8f..b5c1e948d 100644
> --- a/testcases/kernel/syscalls/mbind/mbind01.c
> +++ b/testcases/kernel/syscalls/mbind/mbind01.c
> @@ -34,7 +34,7 @@ static struct bitmask *nodemask, *getnodemask,
> *empty_nodemask;
> static void test_default(unsigned int i, char *p);
> static void test_none(unsigned int i, char *p);
> static void test_invalid_nodemask(unsigned int i, char *p);
> -static void check_policy_pref_no_target(int);
> +static void check_policy_pref_or_local(int);
>
> struct test_case {
> int policy;
> @@ -92,7 +92,7 @@ static struct test_case tcase[] = {
> .ret = 0,
> .err = 0,
> .test = test_none,
> - .check_policy = check_policy_pref_no_target,
> + .check_policy = check_policy_pref_or_local,
> },
> {
> POLICY_DESC(MPOL_PREFERRED),
> @@ -101,6 +101,20 @@ static struct test_case tcase[] = {
> .test = test_default,
> .exp_nodemask = &nodemask,
> },
> + {
> + POLICY_DESC(MPOL_LOCAL),
> + .ret = 0,
> + .err = 0,
> + .test = test_none,
> + .exp_nodemask = &empty_nodemask,
> + .check_policy = check_policy_pref_or_local,
>
This is a bit more permissive, it allows for MPOL_LOCAL to return also
MPOL_PREFERRED.
Shouldn't that still be treated as error?
> + },
> + {
> + POLICY_DESC_TEXT(MPOL_LOCAL, "target exists"),
> + .ret = -1,
> + .err = EINVAL,
> + .test = test_default,
> + },
> {
> POLICY_DESC(UNKNOWN_POLICY),
> .ret = -1,
> @@ -122,7 +136,7 @@ static struct test_case tcase[] = {
> },
> };
>
> -static void check_policy_pref_no_target(int policy)
> +static void check_policy_pref_or_local(int policy)
> {
> if (policy != MPOL_PREFERRED && policy != MPOL_LOCAL) {
> tst_res(TFAIL, "Wrong policy: %s(%d), "
> @@ -182,6 +196,11 @@ static void do_test(unsigned int i)
>
> tst_res(TINFO, "case %s", tc->desc);
>
> + if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
> + tst_res(TCONF, "%s is not supported",
> tst_mempolicy_mode_name(tc->policy));
> + return;
> + }
>
I was thinking of runtime check (to support also downstream kernels that
backported it),
but I don't have strong opinion.
> +
> setup_node();
>
> p = SAFE_MMAP(NULL, MEM_LENGTH, PROT_READ | PROT_WRITE,
> MAP_PRIVATE |
> --
> 2.31.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210729/7098a98b/attachment.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-29 14:20 ` Jan Stancek
@ 2021-07-30 8:03 ` Li Wang
2021-07-30 10:35 ` Jan Stancek
2021-07-30 11:35 ` Feng Tang
0 siblings, 2 replies; 10+ messages in thread
From: Li Wang @ 2021-07-30 8:03 UTC (permalink / raw)
To: ltp
On Thu, Jul 29, 2021 at 10:20 PM Jan Stancek <jstancek@redhat.com> wrote:
>
>
> On Thu, Jul 29, 2021 at 3:25 PM Li Wang <liwang@redhat.com> wrote:
>
>> Signed-off-by: Li Wang <liwang@redhat.com>
>> ---
>> testcases/kernel/syscalls/mbind/mbind01.c | 25 ++++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/mbind/mbind01.c
>> b/testcases/kernel/syscalls/mbind/mbind01.c
>> index d2cf13c8f..b5c1e948d 100644
>> --- a/testcases/kernel/syscalls/mbind/mbind01.c
>> +++ b/testcases/kernel/syscalls/mbind/mbind01.c
>> @@ -34,7 +34,7 @@ static struct bitmask *nodemask, *getnodemask,
>> *empty_nodemask;
>> static void test_default(unsigned int i, char *p);
>> static void test_none(unsigned int i, char *p);
>> static void test_invalid_nodemask(unsigned int i, char *p);
>> -static void check_policy_pref_no_target(int);
>> +static void check_policy_pref_or_local(int);
>>
>> struct test_case {
>> int policy;
>> @@ -92,7 +92,7 @@ static struct test_case tcase[] = {
>> .ret = 0,
>> .err = 0,
>> .test = test_none,
>> - .check_policy = check_policy_pref_no_target,
>> + .check_policy = check_policy_pref_or_local,
>> },
>> {
>> POLICY_DESC(MPOL_PREFERRED),
>> @@ -101,6 +101,20 @@ static struct test_case tcase[] = {
>> .test = test_default,
>> .exp_nodemask = &nodemask,
>> },
>> + {
>> + POLICY_DESC(MPOL_LOCAL),
>> + .ret = 0,
>> + .err = 0,
>> + .test = test_none,
>> + .exp_nodemask = &empty_nodemask,
>> + .check_policy = check_policy_pref_or_local,
>>
>
> This is a bit more permissive, it allows for MPOL_LOCAL to return also
> MPOL_PREFERRED.
> Shouldn't that still be treated as error?
>
To strictly this should be an error.
But I slightly think that it's acceptable to get 'MPOL_PREFERRED' on the old
kernel (i.e. 4.18.0, v5.13) because 'MPOL_LOCAL' is not treated as a real
policy.
And the situation exists for quite a long time.
7858d7bca7fb ("mm/mempolicy: don't handle MPOL_LOCAL like a fake
MPOL_PREFERRED policy")
Without this kernel commit, looks like the MPOL_LOCAL will convert to
MPOL_PREFERRED in mpol_new.
SYSCAL_DEFINE6(mbind, ...)
kernel_mbind
do_mbind
mpol_new
....
# cat mempolicy.c -n
287 /*
288 * MPOL_PREFERRED cannot be used with MPOL_F_STATIC_NODES or
289 * MPOL_F_RELATIVE_NODES if the nodemask is empty (local
allocation).
290 * All other modes require a valid pointer to a non-empty
nodemask.
291 */
292 if (mode == MPOL_PREFERRED) {
293 if (nodes_empty(*nodes)) {
294 if (((flags & MPOL_F_STATIC_NODES) ||
295 (flags & MPOL_F_RELATIVE_NODES)))
296 return ERR_PTR(-EINVAL);
297 }
298 } else if (mode == MPOL_LOCAL) {
299 if (!nodes_empty(*nodes) ||
300 (flags & MPOL_F_STATIC_NODES) ||
301 (flags & MPOL_F_RELATIVE_NODES))
302 return ERR_PTR(-EINVAL);
303 mode = MPOL_PREFERRED; <--------- this line has
been removed after the commit
304 } else if (nodes_empty(*nodes))
305 return ERR_PTR(-EINVAL);
But maybe I was wrong here, CC FengTang in case he has suggestions on this.
>
>> + if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
>> + tst_res(TCONF, "%s is not supported",
>> tst_mempolicy_mode_name(tc->policy));
>> + return;
>> + }
>>
>
> I was thinking of runtime check (to support also downstream kernels that
> backported it),
> but I don't have strong opinion.
>
Thanks, I assume there is little probability to backport it.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210730/782d0cdb/attachment.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-30 8:03 ` Li Wang
@ 2021-07-30 10:35 ` Jan Stancek
2021-07-30 10:44 ` Li Wang
2021-07-30 11:35 ` Feng Tang
1 sibling, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2021-07-30 10:35 UTC (permalink / raw)
To: ltp
On Fri, Jul 30, 2021 at 10:03 AM Li Wang <liwang@redhat.com> wrote:
>
>
> On Thu, Jul 29, 2021 at 10:20 PM Jan Stancek <jstancek@redhat.com> wrote:
>
>>
>>
>> On Thu, Jul 29, 2021 at 3:25 PM Li Wang <liwang@redhat.com> wrote:
>>
>>> Signed-off-by: Li Wang <liwang@redhat.com>
>>> ---
>>> testcases/kernel/syscalls/mbind/mbind01.c | 25 ++++++++++++++++++++---
>>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/mbind/mbind01.c
>>> b/testcases/kernel/syscalls/mbind/mbind01.c
>>> index d2cf13c8f..b5c1e948d 100644
>>> --- a/testcases/kernel/syscalls/mbind/mbind01.c
>>> +++ b/testcases/kernel/syscalls/mbind/mbind01.c
>>> @@ -34,7 +34,7 @@ static struct bitmask *nodemask, *getnodemask,
>>> *empty_nodemask;
>>> static void test_default(unsigned int i, char *p);
>>> static void test_none(unsigned int i, char *p);
>>> static void test_invalid_nodemask(unsigned int i, char *p);
>>> -static void check_policy_pref_no_target(int);
>>> +static void check_policy_pref_or_local(int);
>>>
>>> struct test_case {
>>> int policy;
>>> @@ -92,7 +92,7 @@ static struct test_case tcase[] = {
>>> .ret = 0,
>>> .err = 0,
>>> .test = test_none,
>>> - .check_policy = check_policy_pref_no_target,
>>> + .check_policy = check_policy_pref_or_local,
>>> },
>>> {
>>> POLICY_DESC(MPOL_PREFERRED),
>>> @@ -101,6 +101,20 @@ static struct test_case tcase[] = {
>>> .test = test_default,
>>> .exp_nodemask = &nodemask,
>>> },
>>> + {
>>> + POLICY_DESC(MPOL_LOCAL),
>>> + .ret = 0,
>>> + .err = 0,
>>> + .test = test_none,
>>> + .exp_nodemask = &empty_nodemask,
>>> + .check_policy = check_policy_pref_or_local,
>>>
>>
>> This is a bit more permissive, it allows for MPOL_LOCAL to return also
>> MPOL_PREFERRED.
>> Shouldn't that still be treated as error?
>>
>
> To strictly this should be an error.
>
> But I slightly think that it's acceptable to get 'MPOL_PREFERRED' on the
> old
> kernel (i.e. 4.18.0, v5.13) because 'MPOL_LOCAL' is not treated as a real
> policy.
> And the situation exists for quite a long time.
>
You're right, on older kernel it failed in similar way for MPOL_LOCAL
as it failed for MPOL_PREFERRED on latest one.
Acked-by: Jan Stancek <jstancek@redhat.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210730/18bd325e/attachment.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-30 10:35 ` Jan Stancek
@ 2021-07-30 10:44 ` Li Wang
2021-07-30 11:54 ` Jan Stancek
0 siblings, 1 reply; 10+ messages in thread
From: Li Wang @ 2021-07-30 10:44 UTC (permalink / raw)
To: ltp
On Fri, Jul 30, 2021 at 6:35 PM Jan Stancek <jstancek@redhat.com> wrote:
>
>
> On Fri, Jul 30, 2021 at 10:03 AM Li Wang <liwang@redhat.com> wrote:
>
>>
>>
>> On Thu, Jul 29, 2021 at 10:20 PM Jan Stancek <jstancek@redhat.com> wrote:
>>
>>>
>>>
>>> On Thu, Jul 29, 2021 at 3:25 PM Li Wang <liwang@redhat.com> wrote:
>>>
>>>> Signed-off-by: Li Wang <liwang@redhat.com>
>>>> ---
>>>> testcases/kernel/syscalls/mbind/mbind01.c | 25 ++++++++++++++++++++---
>>>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/testcases/kernel/syscalls/mbind/mbind01.c
>>>> b/testcases/kernel/syscalls/mbind/mbind01.c
>>>> index d2cf13c8f..b5c1e948d 100644
>>>> --- a/testcases/kernel/syscalls/mbind/mbind01.c
>>>> +++ b/testcases/kernel/syscalls/mbind/mbind01.c
>>>> @@ -34,7 +34,7 @@ static struct bitmask *nodemask, *getnodemask,
>>>> *empty_nodemask;
>>>> static void test_default(unsigned int i, char *p);
>>>> static void test_none(unsigned int i, char *p);
>>>> static void test_invalid_nodemask(unsigned int i, char *p);
>>>> -static void check_policy_pref_no_target(int);
>>>> +static void check_policy_pref_or_local(int);
>>>>
>>>> struct test_case {
>>>> int policy;
>>>> @@ -92,7 +92,7 @@ static struct test_case tcase[] = {
>>>> .ret = 0,
>>>> .err = 0,
>>>> .test = test_none,
>>>> - .check_policy = check_policy_pref_no_target,
>>>> + .check_policy = check_policy_pref_or_local,
>>>> },
>>>> {
>>>> POLICY_DESC(MPOL_PREFERRED),
>>>> @@ -101,6 +101,20 @@ static struct test_case tcase[] = {
>>>> .test = test_default,
>>>> .exp_nodemask = &nodemask,
>>>> },
>>>> + {
>>>> + POLICY_DESC(MPOL_LOCAL),
>>>> + .ret = 0,
>>>> + .err = 0,
>>>> + .test = test_none,
>>>> + .exp_nodemask = &empty_nodemask,
>>>> + .check_policy = check_policy_pref_or_local,
>>>>
>>>
>>> This is a bit more permissive, it allows for MPOL_LOCAL to return also
>>> MPOL_PREFERRED.
>>> Shouldn't that still be treated as error?
>>>
>>
>> To strictly this should be an error.
>>
>> But I slightly think that it's acceptable to get 'MPOL_PREFERRED' on the
>> old
>> kernel (i.e. 4.18.0, v5.13) because 'MPOL_LOCAL' is not treated as a
>> real policy.
>> And the situation exists for quite a long time.
>>
>
> You're right, on older kernel it failed in similar way for MPOL_LOCAL
> as it failed for MPOL_PREFERRED on latest one.
>
Or, If we want something more precise, just cancel the
check_policy_pref_or_local on kernel >= 5.14.
Is this sound better?
- if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
- tst_res(TCONF, "%s is not supported",
tst_mempolicy_mode_name(tc->policy));
- return;
+ if (tc->policy == MPOL_LOCAL) {
+ if ((tst_kvercmp(3, 8, 0)) < 0) {
+ tst_res(TCONF, "%s is not supported",
tst_mempolicy_mode_name(tc->policy));
+ return;
+ }
+
+ if ((tst_kvercmp(5, 14, 0)) >= 0)
+ tc->check_policy = NULL;
>
> Acked-by: Jan Stancek <jstancek@redhat.com>
>
>
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210730/aa0582ad/attachment-0001.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-30 8:03 ` Li Wang
2021-07-30 10:35 ` Jan Stancek
@ 2021-07-30 11:35 ` Feng Tang
2021-08-02 2:03 ` Li Wang
1 sibling, 1 reply; 10+ messages in thread
From: Feng Tang @ 2021-07-30 11:35 UTC (permalink / raw)
To: ltp
Hi Li,
On Fri, Jul 30, 2021 at 04:03:28PM +0800, Li Wang wrote:
[....]
> On Thu, Jul 29, 2021 at 10:20 PM Jan Stancek <jstancek@redhat.com> wrote:
> >> @@ -101,6 +101,20 @@ static struct test_case tcase[] = {
> >> .test = test_default,
> >> .exp_nodemask = &nodemask,
> >> },
> >> + {
> >> + POLICY_DESC(MPOL_LOCAL),
> >> + .ret = 0,
> >> + .err = 0,
> >> + .test = test_none,
> >> + .exp_nodemask = &empty_nodemask,
> >> + .check_policy = check_policy_pref_or_local,
> >>
> >
> > This is a bit more permissive, it allows for MPOL_LOCAL to return also
> > MPOL_PREFERRED.
> > Shouldn't that still be treated as error?
> >
>
> To strictly this should be an error.
>
> But I slightly think that it's acceptable to get 'MPOL_PREFERRED' on the old
> kernel (i.e. 4.18.0, v5.13) because 'MPOL_LOCAL' is not treated as a real
> policy.
> And the situation exists for quite a long time.
>
> 7858d7bca7fb ("mm/mempolicy: don't handle MPOL_LOCAL like a fake
> MPOL_PREFERRED policy")
>
> Without this kernel commit, looks like the MPOL_LOCAL will convert to
> MPOL_PREFERRED in mpol_new.
>
> SYSCAL_DEFINE6(mbind, ...)
> kernel_mbind
> do_mbind
> mpol_new
> ....
>
> # cat mempolicy.c -n
>
> 287 /*
> 288 * MPOL_PREFERRED cannot be used with MPOL_F_STATIC_NODES or
> 289 * MPOL_F_RELATIVE_NODES if the nodemask is empty (local
> allocation).
> 290 * All other modes require a valid pointer to a non-empty
> nodemask.
> 291 */
> 292 if (mode == MPOL_PREFERRED) {
> 293 if (nodes_empty(*nodes)) {
> 294 if (((flags & MPOL_F_STATIC_NODES) ||
> 295 (flags & MPOL_F_RELATIVE_NODES)))
> 296 return ERR_PTR(-EINVAL);
> 297 }
> 298 } else if (mode == MPOL_LOCAL) {
> 299 if (!nodes_empty(*nodes) ||
> 300 (flags & MPOL_F_STATIC_NODES) ||
> 301 (flags & MPOL_F_RELATIVE_NODES))
> 302 return ERR_PTR(-EINVAL);
> 303 mode = MPOL_PREFERRED; <--------- this line has
> been removed after the commit
> 304 } else if (nodes_empty(*nodes))
> 305 return ERR_PTR(-EINVAL);
>
> But maybe I was wrong here, CC FengTang in case he has suggestions on this.
Yes, you are right. With the commit MPOL_LOCAL will be taken as a real
policy, be MPOL_LOCAL itself, and not a fake MPOL_PREFERRED.
Faking MPOL_LOCAL as a MPOL_PREFERRED makes the semantic very confusing,
and the kenrel code is also very confusing and difficult to be maintained,
as there are many ambiguous logic to check this faking and transform the
policy back and forth. So we chosed to clean it up.
Thanks,
Feng
>
> >
> >> + if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
> >> + tst_res(TCONF, "%s is not supported",
> >> tst_mempolicy_mode_name(tc->policy));
> >> + return;
> >> + }
> >>
> >
> > I was thinking of runtime check (to support also downstream kernels that
> > backported it),
> > but I don't have strong opinion.
> >
>
> Thanks, I assume there is little probability to backport it.
>
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-30 10:44 ` Li Wang
@ 2021-07-30 11:54 ` Jan Stancek
0 siblings, 0 replies; 10+ messages in thread
From: Jan Stancek @ 2021-07-30 11:54 UTC (permalink / raw)
To: ltp
>
> Or, If we want something more precise, just cancel the
> check_policy_pref_or_local on kernel >= 5.14.
>
> Is this sound better?
>
> - if ((tst_kvercmp(3, 8, 0)) < 0 && (tc->policy == MPOL_LOCAL)) {
> - tst_res(TCONF, "%s is not supported",
> tst_mempolicy_mode_name(tc->policy));
> - return;
>
> + if (tc->policy == MPOL_LOCAL) {
> + if ((tst_kvercmp(3, 8, 0)) < 0) {
> + tst_res(TCONF, "%s is not supported",
> tst_mempolicy_mode_name(tc->policy));
> + return;
> + }
> +
> + if ((tst_kvercmp(5, 14, 0)) >= 0)
> + tc->check_policy = NULL;
>
>
Yes, that works too.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210730/78002607/attachment.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL
2021-07-30 11:35 ` Feng Tang
@ 2021-08-02 2:03 ` Li Wang
0 siblings, 0 replies; 10+ messages in thread
From: Li Wang @ 2021-08-02 2:03 UTC (permalink / raw)
To: ltp
Hi Jan, Feng,
Thanks for your confirmation, the patch was applied.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210802/bfe4861b/attachment.htm>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-02 2:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 13:25 [LTP] [PATCH 1/3] mbind01: make use of tst_numa_mode_name Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 2/3] libs: rename tst_numa_mode_name to tst_mempolicy_mode_name Li Wang
2021-07-29 13:25 ` [LTP] [PATCH 3/3] mbind01: add more tests for MPOL_LOCAL Li Wang
2021-07-29 14:20 ` Jan Stancek
2021-07-30 8:03 ` Li Wang
2021-07-30 10:35 ` Jan Stancek
2021-07-30 10:44 ` Li Wang
2021-07-30 11:54 ` Jan Stancek
2021-07-30 11:35 ` Feng Tang
2021-08-02 2:03 ` Li Wang
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.