kselftest/next build: 6 builds: 0 failed, 6 passed, 1 warning (v6.9-8287-g31a59b76b978)

2024-05-16 Thread kernelci.org bot
kselftest/next build: 6 builds: 0 failed, 6 passed, 1 warning 
(v6.9-8287-g31a59b76b978)

This legacy KernelCI providing this report will shutdown sometime
soon in favor of our new KernelCI infra. Not all tests are being
migrated.
If you are still using this report and want us to prioritize your
usecase in the new system, please let us know at
kerne...@lists.linux.dev

Full Build Summary: 
https://kernelci.org/build/kselftest/branch/next/kernel/v6.9-8287-g31a59b76b978/

Tree: kselftest
Branch: next
Git Describe: v6.9-8287-g31a59b76b978
Git Commit: 31a59b76b9780a9b2d385024e2d6d0d051bb06a5
Git URL: 
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git
Built: 4 unique architectures

Warnings Detected:

arm64:

arm:

i386:

x86_64:
x86_64_defconfig+kselftest (clang-16): 1 warning


Warnings summary:

1vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x23: relocation to 
!ENDBR: .text+0x14dd29



Detailed per-defconfig build reports:


defconfig+kselftest (arm64, gcc-10) — PASS, 0 errors, 0 warnings, 0 section 
mismatches


defconfig+kselftest+arm64-chromebook (arm64, gcc-10) — PASS, 0 errors, 0 
warnings, 0 section mismatches


i386_defconfig+kselftest (i386, gcc-10) — PASS, 0 errors, 0 warnings, 0 section 
mismatches


multi_v7_defconfig+kselftest (arm, gcc-10) — PASS, 0 errors, 0 warnings, 0 
section mismatches


x86_64_defconfig+kselftest (x86_64, gcc-10) — PASS, 0 errors, 0 warnings, 0 
section mismatches


x86_64_defconfig+kselftest (x86_64, clang-16) — PASS, 0 errors, 1 warning, 0 
section mismatches

Warnings:
vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x23: relocation to !ENDBR: 
.text+0x14dd29

---
For more info write to 



Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Shuah Khan

On 5/16/24 12:05, Shuah Khan wrote:

On 5/16/24 11:45, Tejun Heo wrote:

Hello,

On Thu, May 16, 2024 at 10:31:13AM -0600, Shuah Khan wrote:

I am exploring options and leaning towards reverting the patch

daef47b89efd ("selftests: Compile kselftest headers with -D_GNU_SOURCE")

Your amending the PR helps me if I have to send revert. I am sorry
for the trouble.

I can all of them together in a second update or after the merge window
closes.


The cgroup commit is already pulled in unfortunately. Can you please handle
the revert and whatever's necessary to fix up the situation? I'll ask you
what to do with selftest patches from now on.



Thanks for the update. Yes I am working on fixing the situation and
will send revert for cgroup test patch as well if necessary.

No worries. It is not a problem for you to handle cgroup test patches
in general. I will need your review anyway and letting you handle them
reduces the overhead.

This kind of framework change causes needs to be coordinated.
I should have held back on the framework change on my part.



As mentioned in the other thread

https://lore.kernel.org/linux-kselftest/24975952-b1fa-44a5-bac5-aef538ad0...@linuxfoundation.org/T/#t

I reverted the following patch and the framework change patch.
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=next

Will send PR to Linus this weekend.

"selftests/cgroup: Drop define _GNU_SOURCE"
commit c1457d9aad5ee2feafcf85aa9a58ab50500159d2

thanks,
-- Shuah



Re: -D_GNU_SOURCE kselftest breakage in mainline

2024-05-16 Thread Shuah Khan

On 5/16/24 09:09, Mark Brown wrote:

On Thu, May 16, 2024 at 08:53:52AM -0600, Shuah Khan wrote:

On 5/16/24 08:02, Mark Brown wrote:



I'm seeing quite a lot of breakage in mainline as a result of
daef47b89efd0b7 ("selftests: Compile kselftest headers with
-D_GNU_SOURCE") and daef47b89efd0 ("selftests: Compile kselftest headers
with -D_GNU_SOURCE") - thus far I've found that the use of
static_assert() is triggering build breaks where testsuites aren't
picking up the addition of _GNU_SOURCE (including stopping installing
the other tests in the same directory), and there's a bunch of tests
which #define _GNU_SOURCE in their code and now trigger build warnings.
I'm looking at fixes and mitigations now.



Would it be better to revert this for now and get this for now? I wouldn't
want you to extra busy work to workaround this.





I determined the best option now is reverting the 3 patches.

commit daef47b89efd0b745e8478d69a3ad724bd8b4dc6
"selftests: Compile kselftest headers with -D_GNU_SOURCE"

"selftests/sgx: Include KHDR_INCLUDES in Makefile"
commit 2c3b8f8f37c6c0c926d584cf4158db95e62b960c

"selftests/cgroup: Drop define _GNU_SOURCE"
commit c1457d9aad5ee2feafcf85aa9a58ab50500159d2

I have this in linux-kselftest next now and will send pull
request to Linus this weekend.

Tejun, I made sure cgroup compiles after reverting the framework
patch and the cgroup test patch.

Mark,

Thank you Mark for finding the problem and sending in mitigation
fixes. I will hold off on your alsa patch:

kselftest/alsa: Ensure _GNU_SOURCE is defined
https://patchwork.kernel.org/project/linux-kselftest/patch/20240516-kselftest-fix-gnu-source-v1-1-e482ca6bf...@kernel.org/

There is no need to for the following patch

kselftest: Desecalate reporting of missing _GNU_SOURCE
https://patchwork.kernel.org/project/linux-kselftest/patch/20240516-kselftest-mitigate-gnu-source-v1-1-a0e814ff2...@kernel.org/

thanks,
-- Shuah



Re: [PATCH net] selftests/net: reduce xfrm_policy test time

2024-05-16 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski :

On Tue, 14 May 2024 17:52:27 +0800 you wrote:
> The check_random_order test add/get plenty of xfrm rules, which consume
> a lot time on debug kernel and always TIMEOUT. Let's reduce the test
> loop and see if it works.
> 
> Signed-off-by: Hangbin Liu 
> ---
>  tools/testing/selftests/net/xfrm_policy.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [net] selftests/net: reduce xfrm_policy test time
https://git.kernel.org/netdev/net/c/988af276360b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html





[PATCH] selftest: rtc: Add to check rtc alarm status for alarm related test

2024-05-16 Thread Joseph Jang
In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
code. This design may miss detecting real problems when the
efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.

In order to make rtctest more explicit and robust, we propose to use
RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
running alarm related tests. If the kernel does not support RTC_PARAM_GET
ioctl interface, we will fallback to check the presence of "alarm" in
/proc/driver/rtc.

The rtctest requires the read permission on /dev/rtc0. The rtctest will
be skipped if the /dev/rtc0 is not readable.

Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
as optional")

Reviewed-by: Jeremy Szu 
Reviewed-by: Matthew R. Ochs 
Signed-off-by: Joseph Jang 
---
 tools/testing/selftests/rtc/Makefile  |  2 +-
 tools/testing/selftests/rtc/rtctest.c | 72 +++
 2 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/rtc/Makefile 
b/tools/testing/selftests/rtc/Makefile
index 55198ecc04db..6e3a98fb24ba 100644
--- a/tools/testing/selftests/rtc/Makefile
+++ b/tools/testing/selftests/rtc/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall
+CFLAGS += -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include/
 LDLIBS += -lrt -lpthread -lm
 
 TEST_GEN_PROGS = rtctest
diff --git a/tools/testing/selftests/rtc/rtctest.c 
b/tools/testing/selftests/rtc/rtctest.c
index 63ce02d1d5cc..aa47b17fbd1a 100644
--- a/tools/testing/selftests/rtc/rtctest.c
+++ b/tools/testing/selftests/rtc/rtctest.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -24,12 +25,17 @@
 #define READ_LOOP_SLEEP_MS 11
 
 static char *rtc_file = "/dev/rtc0";
+static char *rtc_procfs = "/proc/driver/rtc";
 
 FIXTURE(rtc) {
int fd;
 };
 
 FIXTURE_SETUP(rtc) {
+   if (access(rtc_file, R_OK) != 0)
+   SKIP(return, "Skipping test since cannot access %s, perhaps 
miss sudo",
+rtc_file);
+
self->fd = open(rtc_file, O_RDONLY);
 }
 
@@ -82,6 +88,36 @@ static void nanosleep_with_retries(long ns)
}
 }
 
+static bool is_rtc_alarm_supported(int fd)
+{
+   struct rtc_param param = { 0 };
+   int rc;
+   char buf[1024] = { 0 };
+
+   /* Validate kernel reflects unsupported RTC alarm state */
+   param.param = RTC_PARAM_FEATURES;
+   param.index = 0;
+   rc = ioctl(fd, RTC_PARAM_GET, );
+   if (rc < 0) {
+   /* Fallback to read rtc procfs */
+   fd = open(rtc_procfs, O_RDONLY);
+   if (fd != -1) {
+   rc = read(fd, buf, sizeof(buf));
+   close(fd);
+
+   /* Check for the presence of "alarm" in the buf */
+   if (strstr(buf, "alarm") == NULL)
+   return false;
+   } else
+   return false;
+   } else {
+   if ((param.uvalue & _BITUL(RTC_FEATURE_ALARM)) == 0)
+   return false;
+   }
+
+   return true;
+}
+
 TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
int rc;
long iter_count = 0;
@@ -202,6 +238,9 @@ TEST_F(rtc, alarm_alm_set) {
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);
 
+   if (!is_rtc_alarm_supported(self->fd))
+   SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, );
ASSERT_NE(-1, rc);
 
@@ -209,11 +248,7 @@ TEST_F(rtc, alarm_alm_set) {
gmtime_r(, (struct tm *));
 
rc = ioctl(self->fd, RTC_ALM_SET, );
-   if (rc == -1) {
-   ASSERT_EQ(EINVAL, errno);
-   TH_LOG("skip alarms are not supported.");
-   return;
-   }
+   ASSERT_NE(-1, rc);
 
rc = ioctl(self->fd, RTC_ALM_READ, );
ASSERT_NE(-1, rc);
@@ -260,6 +295,9 @@ TEST_F(rtc, alarm_wkalm_set) {
SKIP(return, "Skipping test since %s does not exist", rtc_file);
ASSERT_NE(-1, self->fd);
 
+   if (!is_rtc_alarm_supported(self->fd))
+   SKIP(return, "Skipping test since alarms are not supported.");
+
rc = ioctl(self->fd, RTC_RD_TIME, );
ASSERT_NE(-1, rc);
 
@@ -269,11 +307,7 @@ TEST_F(rtc, alarm_wkalm_set) {
alarm.enabled = 1;
 
rc = ioctl(self->fd, RTC_WKALM_SET, );
-   if (rc == -1) {
-   ASSERT_EQ(EINVAL, errno);
-   TH_LOG("skip alarms are not supported.");
-   return;
-   }
+   ASSERT_NE(-1, rc);
 
rc = ioctl(self->fd, RTC_WKALM_RD, );

[PATCHv2 net] selftests/net: use tc rule to filter the na packet

2024-05-16 Thread Hangbin Liu
Test arp_ndisc_untracked_subnets use tcpdump to filter the unsolicited
and untracked na messages. It set -e before calling tcpdump. But if
tcpdump filters 0 packet, it will return none zero, and cause the script
to exit.

Instead of using slow tcpdump to capture packets, let's using tc rule
to filter out the na message.

At the same time, fix function setup_v6 which only needs one parameter.
Move all the related helpers from forwarding lib.sh to net lib.sh.

Fixes: 0ea7b0a454ca ("selftests: net: arp_ndisc_untracked_subnets: test for 
arp_accept and accept_untracked_na")
Signed-off-by: Hangbin Liu 
---

v2: rebase to latest net main code. no update,
v1: https://lore.kernel.org/netdev/20240514071130.2121042-1-liuhang...@gmail.com

---
 .../net/arp_ndisc_untracked_subnets.sh| 53 ++---
 tools/testing/selftests/net/forwarding/lib.sh | 58 ---
 tools/testing/selftests/net/lib.sh| 58 +++
 3 files changed, 75 insertions(+), 94 deletions(-)

diff --git a/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh 
b/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh
index a40c0e9bd023..eef5cbf6eecc 100755
--- a/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh
+++ b/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh
@@ -73,25 +73,19 @@ setup_v6() {
# namespaces. veth0 is veth-router, veth1 is veth-host.
# first, set up the inteface's link to the namespace
# then, set the interface "up"
-   ip -6 -netns ${ROUTER_NS_V6} link add name ${ROUTER_INTF} \
-   type veth peer name ${HOST_INTF}
-
-   ip -6 -netns ${ROUTER_NS_V6} link set dev ${ROUTER_INTF} up
-   ip -6 -netns ${ROUTER_NS_V6} link set dev ${HOST_INTF} netns \
-   ${HOST_NS_V6}
+   ip -n ${ROUTER_NS_V6} link add name ${ROUTER_INTF} \
+   type veth peer name ${HOST_INTF} netns ${HOST_NS_V6}
 
-   ip -6 -netns ${HOST_NS_V6} link set dev ${HOST_INTF} up
-   ip -6 -netns ${ROUTER_NS_V6} addr add \
-   ${ROUTER_ADDR_V6}/${PREFIX_WIDTH_V6} dev ${ROUTER_INTF} nodad
+   # Add tc rule to filter out host na message
+   tc -n ${ROUTER_NS_V6} qdisc add dev ${ROUTER_INTF} clsact
+   tc -n ${ROUTER_NS_V6} filter add dev ${ROUTER_INTF} \
+   ingress protocol ipv6 pref 1 handle 101 \
+   flower src_ip ${HOST_ADDR_V6} ip_proto icmpv6 type 136 skip_hw 
action pass
 
HOST_CONF=net.ipv6.conf.${HOST_INTF}
ip netns exec ${HOST_NS_V6} sysctl -qw ${HOST_CONF}.ndisc_notify=1
ip netns exec ${HOST_NS_V6} sysctl -qw ${HOST_CONF}.disable_ipv6=0
-   ip -6 -netns ${HOST_NS_V6} addr add ${HOST_ADDR_V6}/${PREFIX_WIDTH_V6} \
-   dev ${HOST_INTF}
-
ROUTER_CONF=net.ipv6.conf.${ROUTER_INTF}
-
ip netns exec ${ROUTER_NS_V6} sysctl -w \
${ROUTER_CONF}.forwarding=1 >/dev/null 2>&1
ip netns exec ${ROUTER_NS_V6} sysctl -w \
@@ -99,6 +93,13 @@ setup_v6() {
ip netns exec ${ROUTER_NS_V6} sysctl -w \
${ROUTER_CONF}.accept_untracked_na=${accept_untracked_na} \
>/dev/null 2>&1
+
+   ip -n ${ROUTER_NS_V6} link set dev ${ROUTER_INTF} up
+   ip -n ${HOST_NS_V6} link set dev ${HOST_INTF} up
+   ip -n ${ROUTER_NS_V6} addr add ${ROUTER_ADDR_V6}/${PREFIX_WIDTH_V6} \
+   dev ${ROUTER_INTF} nodad
+   ip -n ${HOST_NS_V6} addr add ${HOST_ADDR_V6}/${PREFIX_WIDTH_V6} \
+   dev ${HOST_INTF}
set +e
 }
 
@@ -162,26 +163,6 @@ arp_test_gratuitous_combinations() {
arp_test_gratuitous 2 1
 }
 
-cleanup_tcpdump() {
-   set -e
-   [[ ! -z  ${tcpdump_stdout} ]] && rm -f ${tcpdump_stdout}
-   [[ ! -z  ${tcpdump_stderr} ]] && rm -f ${tcpdump_stderr}
-   tcpdump_stdout=
-   tcpdump_stderr=
-   set +e
-}
-
-start_tcpdump() {
-   set -e
-   tcpdump_stdout=`mktemp`
-   tcpdump_stderr=`mktemp`
-   ip netns exec ${ROUTER_NS_V6} timeout 15s \
-   tcpdump --immediate-mode -tpni ${ROUTER_INTF} -c 1 \
-   "icmp6 && icmp6[0] == 136 && src ${HOST_ADDR_V6}" \
-   > ${tcpdump_stdout} 2> /dev/null
-   set +e
-}
-
 verify_ndisc() {
local accept_untracked_na=$1
local same_subnet=$2
@@ -222,8 +203,9 @@ ndisc_test_untracked_advertisements() {
HOST_ADDR_V6=2001:db8:abcd:0012::3
fi
fi
-   setup_v6 $1 $2
-   start_tcpdump
+   setup_v6 $1
+   slowwait_for_counter 15 1 \
+   tc_rule_handle_stats_get "dev ${ROUTER_INTF} ingress" 101 
".packets" "-n ${ROUTER_NS_V6}"
 
if verify_ndisc $1 $2; then
printf "TEST: %-60s  [ OK ]\n" "${test_msg[*]}"
@@ -231,7 +213,6 @@ ndisc_test_untracked_advertisements() {
printf "TEST: %-60s  [FAIL]\n" "${test_msg[*]}"
fi
 
-   cleanup_tcpdump
cleanup_v6
set +e
 }
diff --git 

Re: [PATCH v4 00/10] clk: Add kunit tests for fixed rate and parent data

2024-05-16 Thread Stephen Boyd
Quoting Rob Herring (2024-05-15 15:08:47)
> On Wed, May 15, 2024 at 4:15 PM Stephen Boyd  wrote:
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 389d4ea6bfc1..acecefcfdba7 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -421,6 +421,7 @@ int of_platform_bus_probe(struct device_node *root,
> > if (of_match_node(matches, root)) {
> > rc = of_platform_bus_create(root, matches, NULL, parent, 
> > false);
> > } else for_each_child_of_node(root, child) {
> > +   of_node_set_flag(root, OF_POPULATED_BUS);
> 
> No, the same spot as of_platform_populate has it. I guess this would
> be the same, but no reason to do this in the for_each_child_of_node
> loop...

Ok. I'm not intending to send this patch.

> 
> > if (!of_match_node(matches, child))
> > continue;
> > rc = of_platform_bus_create(child, matches, NULL, parent, 
> > false);
> >
> >
> > This doesn't work though. I see that prom_init() is called, which
> > constructs a DTB and flattens it to be unflattened by
> > unflatten_device_tree(). The powerpc machine type used by qemu is
> > PLATFORM_PSERIES_LPAR. It looks like it never calls
> > of_platform_bus_probe() from the pseries platform code.
> 
> Huh. Maybe pseries doesn't have any platform devices?

Looks like it.

> 
> Ideally, we'd still do it in of_platform_default_populate_init(), but
> if you look at the history, you'll see that broke some PPC boards
> (damn initcall ordering).
> 
> > What about skipping the OF_POPULATED_BUS check, or skipping the check
> > when the parent is the root node? This is the if condition that's
> > giving the headache.
> 
> I don't think we should just remove it, but a root node check seems fine.
> 

Alright. I've added a check to see if the root node is the parent to
allow it. That works well enough, so I'll send that in v5.



Re: [PATCH] kselftest: Desecalate reporting of missing _GNU_SOURCE

2024-05-16 Thread Kees Cook
On Thu, May 16, 2024 at 04:28:48PM +0100, Mark Brown wrote:
> Commit daef47b89efd0b7 ("selftests: Compile kselftest headers with
> -D_GNU_SOURCE") adds a static_assert() which means that things which
> would be warnings about undeclared functions get escalated into build
> failures.  While we do actually want _GNU_SOURCE to be defined for users
> of kselftest_harness we haven't actually done that yet and this is
> causing widespread build breaks which were previously warnings about
> uses of asprintf() without prototypes, including causing other test
> programs in the same directory to fail to build.
> 
> Since the build failures that are introduced cause additional issues due
> to make stopping builds early replace the static_assert() with a
> missing without making the error more severe than it already was.  This
> will be moot once the issue is fixed properly but reduces the disruption
> while that happens.
> 
> Signed-off-by: Mark Brown 

Reviewed-by: Kees Cook 

-- 
Kees Cook



[PATCH v5] kunit: Cover 'assert.c' with tests

2024-05-16 Thread Ivan Orlov
There are multiple assertion formatting functions in the `assert.c`
file, which are not covered with tests yet. Implement the KUnit test
for these functions.

The test consists of 11 test cases for the following functions:

1) 'is_literal'
2) 'is_str_literal'
3) 'kunit_assert_prologue', test case for multiple assert types
4) 'kunit_assert_print_msg'
5) 'kunit_unary_assert_format'
6) 'kunit_ptr_not_err_assert_format'
7) 'kunit_binary_assert_format'
8) 'kunit_binary_ptr_assert_format'
9) 'kunit_binary_str_assert_format'
10) 'kunit_assert_hexdump'
11) 'kunit_mem_assert_format'

The test aims at maximizing the branch coverage for the assertion
formatting functions.

As you can see, it covers some of the static helper functions as
well, so mark the static functions in `assert.c` as 'VISIBLE_IF_KUNIT'
and conditionally export them with EXPORT_SYMBOL_IF_KUNIT. Add the
corresponding definitions to `assert.h`.

Build the assert test when CONFIG_KUNIT_TEST is enabled, similar to
how it is done for the string stream test.

Signed-off-by: Ivan Orlov 
---
V1 -> V2:
- Check the output from the string stream for containing the key parts
instead of comparing the results with expected strings char by char, as
it was suggested by Rae Moar . Define two macros to
make it possible (ASSERT_TEST_EXPECT_CONTAIN and
ASSERT_TEST_EXPECT_NCONTAIN).
- Mark the static functions in `assert.c` as VISIBLE_IF_KUNIT and export
them conditionally if kunit is enabled instead of including the
`assert_test.c` file in the end of `assert.c`. This way we will decouple
the test from the implementation (SUT).
- Update the kunit_assert_hexdump test: now it checks for presense of
the brackets '<>' around the non-matching bytes, instead of comparing
the kunit_assert_hexdump output char by char.
V2 -> V3:
- Make test case array and test suite definitions static
- Change the condition in `assert.h`: we should declare VISIBLE_IF_KUNIT
functions in the header file when CONFIG_KUNIT is enabled, not
CONFIG_KUNIT_TEST. Otherwise, if CONFIG_KUNIT_TEST is disabled,
VISIBLE_IF_KUNIT functions in the `assert.c` are not static, and
prototypes for them can't be found.
- Add MODULE_LICENSE and MODULE_DESCRIPTION macros
V3 -> V4:
- Compile the assertion test only when CONFIG_KUNIT_TEST is set to 'y',
as it is done for the string-stream test. It is necessary since
functions from the string-stream are not exported into the public
namespace, and therefore they are not accessible when linking and
loading the test module.
V4 -> V5:
- Remove EXPORT_SYMBOL_IF_KUNIT from the assert.c and all of the
module-related macros from the test source since the test can't
be used as a loadable module for now (see V3 -> V4).

 include/kunit/assert.h  |  11 ++
 lib/kunit/Makefile  |   1 +
 lib/kunit/assert.c  |  19 +-
 lib/kunit/assert_test.c | 388 
 4 files changed, 411 insertions(+), 8 deletions(-)
 create mode 100644 lib/kunit/assert_test.c

diff --git a/include/kunit/assert.h b/include/kunit/assert.h
index 24c2b9fa61e8..7e7490a74b13 100644
--- a/include/kunit/assert.h
+++ b/include/kunit/assert.h
@@ -218,4 +218,15 @@ void kunit_mem_assert_format(const struct kunit_assert 
*assert,
 const struct va_format *message,
 struct string_stream *stream);
 
+#if IS_ENABLED(CONFIG_KUNIT)
+void kunit_assert_print_msg(const struct va_format *message,
+   struct string_stream *stream);
+bool is_literal(const char *text, long long value);
+bool is_str_literal(const char *text, const char *value);
+void kunit_assert_hexdump(struct string_stream *stream,
+ const void *buf,
+ const void *compared_buf,
+ const size_t len);
+#endif
+
 #endif /*  _KUNIT_ASSERT_H */
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 309659a32a78..900e6447c8e8 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_KUNIT_TEST) +=   kunit-test.o
 # string-stream-test compiles built-in only.
 ifeq ($(CONFIG_KUNIT_TEST),y)
 obj-$(CONFIG_KUNIT_TEST) +=string-stream-test.o
+obj-$(CONFIG_KUNIT_TEST) +=assert_test.o
 endif
 
 obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=kunit-example-test.o
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index dd1d633d0fe2..867aa5c4bccf 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -7,6 +7,7 @@
  */
 #include 
 #include 
+#include 
 
 #include "string-stream.h"
 
@@ -30,8 +31,9 @@ void kunit_assert_prologue(const struct kunit_loc *loc,
 }
 EXPORT_SYMBOL_GPL(kunit_assert_prologue);
 
-static void kunit_assert_print_msg(const struct va_format *message,
-  struct string_stream *stream)
+VISIBLE_IF_KUNIT
+void kunit_assert_print_msg(const struct va_format *message,
+   struct string_stream *stream)
 {
if (message->fmt)

Re: [PATCH v6 03/17] riscv: vector: Use vlenb from DT

2024-05-16 Thread Conor Dooley
On Thu, May 16, 2024 at 01:28:45PM -0700, Charlie Jenkins wrote:
> On Thu, May 16, 2024 at 05:24:25PM +0100, Conor Dooley wrote:
> > On Thu, May 16, 2024 at 10:00:12PM +0800, Andy Chiu wrote:
> > > On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins  
> > > wrote:
> > 
> > > > +   if (elf_hwcap & COMPAT_HWCAP_ISA_V && 
> > > > has_riscv_homogeneous_vlenb() < 0) {
> > > > +   pr_warn("Unsupported heterogeneous vlen 
> > > > detected, vector extension disabled.\
> > > > +   elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> > > > +   }
> > > 
> > > We only touch COMPAT_HWCAP_ISA_V and the failed case only turns off the
> > > rectified V. So here we have nothing to do with the Xtheadvector.
> > 
> > There's nothing t-head related in the tree at this point, so doing
> > anything with it would cause build issues.
> > 
> > > However, I am still confused because I think Xtheadvector would also
> > > need to call into this check, so as to setup vlenb.
> > 
> > 
> > > Apart from that, it seems like some vendor stating Xtheadvector is
> > > actually vector-0.7.
> > 
> > The T-Head implementation is 0.7.x, but I am not really sure what you
> > mean by this comment.
> 
> Andy, the idea of this patch was to be able to support this binding on
> more than just xtheadvector.
> 
> You are correct though Andy, this is a problem that a later patch in
> this series doesn't disable xtheadvector when vlenb is not homogeneous.
> I am going to wait to send out any more versions until after this merge
> window but I will fix this in the next version. Thank you! 

Agreed on all counts :)

> > > Please correct me if I speak anything wrong. One
> > > thing I noticed is that Xtheadvector wouldn't trap on reading
> > > th.vlenb but vector-0.7 would. If that is the case, should we require
> > > Xtheadvector to specify `riscv,vlenb` on the device tree?
> > 
> > In the world of Linux, "vector-0.7" isn't a thing. There's only 1.0, and
> > after this patchset, "xtheadvector". My understanding, from discussion
> > on earlier versions of this series the trap is actually accessing
> > th.vlenb register, despite the documentation stating that it is
> > unprivileged:
> > https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
> > I assume Charlie tried it but was trapping, as v1 had a comment:
> > +* Although xtheadvector states that th.vlenb exists and
> > +* overlaps with the vector 1.0 extension overlaps, an illegal
> > +* instruction is raised if read. These systems all currently
> > +* have a fixed vector length of 128, so hardcode that value.
> 
> On my board with a c906 attempting to read th.vlenb (which is supposed
> to have the same encoding as vlenb) raises an illegal instruction
> exception from S-mode even though the documentation states that it
> shouldn't. Because the documentation states that vlenb is available, I
> haven't made it required for xtheadvector, I am not sure the proper
> solution for that.

Would you mind raising an issue on the T-Head extension spec repo about
this?

Thanks,
Conor.


signature.asc
Description: PGP signature


Re: [PATCH v6 03/17] riscv: vector: Use vlenb from DT

2024-05-16 Thread Charlie Jenkins
On Thu, May 16, 2024 at 05:24:25PM +0100, Conor Dooley wrote:
> On Thu, May 16, 2024 at 10:00:12PM +0800, Andy Chiu wrote:
> > On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins  wrote:
> 
> > > +   if (elf_hwcap & COMPAT_HWCAP_ISA_V && 
> > > has_riscv_homogeneous_vlenb() < 0) {
> > > +   pr_warn("Unsupported heterogeneous vlen detected, 
> > > vector extension disabled.\
> > > +   elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> > > +   }
> > 
> > We only touch COMPAT_HWCAP_ISA_V and the failed case only turns off the
> > rectified V. So here we have nothing to do with the Xtheadvector.
> 
> There's nothing t-head related in the tree at this point, so doing
> anything with it would cause build issues.
> 
> > However, I am still confused because I think Xtheadvector would also
> > need to call into this check, so as to setup vlenb.
> 
> 
> > Apart from that, it seems like some vendor stating Xtheadvector is
> > actually vector-0.7.
> 
> The T-Head implementation is 0.7.x, but I am not really sure what you
> mean by this comment.

Andy, the idea of this patch was to be able to support this binding on
more than just xtheadvector.

You are correct though Andy, this is a problem that a later patch in
this series doesn't disable xtheadvector when vlenb is not homogeneous.
I am going to wait to send out any more versions until after this merge
window but I will fix this in the next version. Thank you! 

> 
> > Please correct me if I speak anything wrong. One
> > thing I noticed is that Xtheadvector wouldn't trap on reading
> > th.vlenb but vector-0.7 would. If that is the case, should we require
> > Xtheadvector to specify `riscv,vlenb` on the device tree?
> 
> In the world of Linux, "vector-0.7" isn't a thing. There's only 1.0, and
> after this patchset, "xtheadvector". My understanding, from discussion
> on earlier versions of this series the trap is actually accessing
> th.vlenb register, despite the documentation stating that it is
> unprivileged:
> https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
> I assume Charlie tried it but was trapping, as v1 had a comment:
> +  * Although xtheadvector states that th.vlenb exists and
> +  * overlaps with the vector 1.0 extension overlaps, an illegal
> +  * instruction is raised if read. These systems all currently
> +  * have a fixed vector length of 128, so hardcode that value.

On my board with a c906 attempting to read th.vlenb (which is supposed
to have the same encoding as vlenb) raises an illegal instruction
exception from S-mode even though the documentation states that it
shouldn't. Because the documentation states that vlenb is available, I
haven't made it required for xtheadvector, I am not sure the proper
solution for that.

- Charlie

> 
> Cheers,
> Conor.





Re: [PATCH v4] kunit: Cover 'assert.c' with tests

2024-05-16 Thread Ivan Orlov

On 5/16/24 19:57, Rae Moar wrote:

On Wed, May 15, 2024 at 10:20 AM Ivan Orlov  wrote:


There are multiple assertion formatting functions in the `assert.c`
file, which are not covered with tests yet. Implement the KUnit test
for these functions.

The test consists of 11 test cases for the following functions:

1) 'is_literal'
2) 'is_str_literal'
3) 'kunit_assert_prologue', test case for multiple assert types
4) 'kunit_assert_print_msg'
5) 'kunit_unary_assert_format'
6) 'kunit_ptr_not_err_assert_format'
7) 'kunit_binary_assert_format'
8) 'kunit_binary_ptr_assert_format'
9) 'kunit_binary_str_assert_format'
10) 'kunit_assert_hexdump'
11) 'kunit_mem_assert_format'

The test aims at maximizing the branch coverage for the assertion
formatting functions.

As you can see, it covers some of the static helper functions as
well, so mark the static functions in `assert.c` as 'VISIBLE_IF_KUNIT'
and conditionally export them with EXPORT_SYMBOL_IF_KUNIT. Add the
corresponding definitions to `assert.h`.

Build the assert test when CONFIG_KUNIT_TEST is enabled, similar to
how it is done for the string stream test.

Signed-off-by: Ivan Orlov 
---
V1 -> V2:
- Check the output from the string stream for containing the key parts
instead of comparing the results with expected strings char by char, as
it was suggested by Rae Moar . Define two macros to
make it possible (ASSERT_TEST_EXPECT_CONTAIN and
ASSERT_TEST_EXPECT_NCONTAIN).
- Mark the static functions in `assert.c` as VISIBLE_IF_KUNIT and export
them conditionally if kunit is enabled instead of including the
`assert_test.c` file in the end of `assert.c`. This way we will decouple
the test from the implementation (SUT).
- Update the kunit_assert_hexdump test: now it checks for presense of
the brackets '<>' around the non-matching bytes, instead of comparing
the kunit_assert_hexdump output char by char.
V2 -> V3:
- Make test case array and test suite definitions static
- Change the condition in `assert.h`: we should declare VISIBLE_IF_KUNIT
functions in the header file when CONFIG_KUNIT is enabled, not
CONFIG_KUNIT_TEST. Otherwise, if CONFIG_KUNIT_TEST is disabled,
VISIBLE_IF_KUNIT functions in the `assert.c` are not static, and
prototypes for them can't be found.
- Add MODULE_LICENSE and MODULE_DESCRIPTION macros
V3 -> V4:
- Compile the assertion test only when CONFIG_KUNIT_TEST is set to 'y',
as it is done for the string-stream test. It is necessary since
functions from the string-stream are not exported into the public
namespace, and therefore they are not accessible when linking and
loading the test module.


Hi Ivan!

This looks great! Just one last thing, since this test is no longer
loadable as a module, could you remove the exporting of new symbols
and adding of the module license. Those can be part of the next patch,
where we convert these tests to modules.

Thanks!
-Rae



Hi Rae,

Ah, sorry, I completely forgot to remove the module-related part. Thank 
you for the review, and I'll try to be more attentive next time :)


--
Kind regards,
Ivan Orlov




Re: [PATCH v1 7/7] selftest: test system-wide workingset reporting

2024-05-16 Thread Muhammad Usama Anjum
On 5/4/24 1:30 AM, Yuanchu Xie wrote:
> A basic test that verifies the working set size of a simple memory
> accessor. It should work with or without the aging thread.
> 
> Question: I don't know how to best test file memory in selftests. Is
> there a place where I should put the temporary file? /tmp can be tmpfs
> mounted in many distros.
> 
> Signed-off-by: Yuanchu Xie 
> ---
>  tools/testing/selftests/mm/.gitignore |   1 +
>  tools/testing/selftests/mm/Makefile   |   3 +
>  .../testing/selftests/mm/workingset_report.c  | 317 +
>  .../testing/selftests/mm/workingset_report.h  |  39 ++
>  .../selftests/mm/workingset_report_test.c | 332 ++
Add this test to run_vmtests.sh as all the tests are run from this script.

>  5 files changed, 692 insertions(+)
>  create mode 100644 tools/testing/selftests/mm/workingset_report.c
>  create mode 100644 tools/testing/selftests/mm/workingset_report.h
>  create mode 100644 tools/testing/selftests/mm/workingset_report_test.c
> 
> diff --git a/tools/testing/selftests/mm/.gitignore 
> b/tools/testing/selftests/mm/.gitignore
> index 4ff10ea61461..14a2412c8257 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -46,3 +46,4 @@ gup_longterm
>  mkdirty
>  va_high_addr_switch
>  hugetlb_fault_after_madv
> +workingset_report_test
> diff --git a/tools/testing/selftests/mm/Makefile 
> b/tools/testing/selftests/mm/Makefile
> index 2453add65d12..c0869bf07e99 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -70,6 +70,7 @@ TEST_GEN_FILES += ksm_tests
>  TEST_GEN_FILES += ksm_functional_tests
>  TEST_GEN_FILES += mdwe_test
>  TEST_GEN_FILES += hugetlb_fault_after_madv
> +TEST_GEN_FILES += workingset_report_test
>  
>  ifneq ($(ARCH),arm64)
>  TEST_GEN_FILES += soft-dirty
> @@ -123,6 +124,8 @@ $(TEST_GEN_FILES): vm_util.c thp_settings.c
>  $(OUTPUT)/uffd-stress: uffd-common.c
>  $(OUTPUT)/uffd-unit-tests: uffd-common.c
>  
> +$(OUTPUT)/workingset_report_test: workingset_report.c
> +
>  ifeq ($(ARCH),x86_64)
>  BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
>  BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
> diff --git a/tools/testing/selftests/mm/workingset_report.c 
> b/tools/testing/selftests/mm/workingset_report.c
> new file mode 100644
> index ..0d744bae5432
> --- /dev/null
> +++ b/tools/testing/selftests/mm/workingset_report.c
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "workingset_report.h"
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../kselftest.h"
> +
> +#define SYSFS_NODE_ONLINE "/sys/devices/system/node/online"
> +#define PROC_DROP_CACHES "/proc/sys/vm/drop_caches"
> +
> +/* Returns read len on success, or -errno on failure. */
> +static ssize_t read_text(const char *path, char *buf, size_t max_len)
> +{
> + ssize_t len;
> + int fd, err;
> + size_t bytes_read = 0;
> +
> + if (!max_len)
> + return -EINVAL;
> +
> + fd = open(path, O_RDONLY);
> + if (fd < 0)
> + return -errno;
> +
> + while (bytes_read < max_len - 1) {
> + len = read(fd, buf + bytes_read, max_len - 1 - bytes_read);
> +
> + if (len <= 0)
> + break;
> + bytes_read += len;
> + }
> +
> + buf[bytes_read] = '\0';
> +
> + err = -errno;
> + close(fd);
> + return len < 0 ? err : bytes_read;
> +}
> +
> +/* Returns written len on success, or -errno on failure. */
> +static ssize_t write_text(const char *path, const char *buf, ssize_t max_len)
> +{
> + int fd, len, err;
> + size_t bytes_written = 0;
> +
> + fd = open(path, O_WRONLY | O_APPEND);
> + if (fd < 0)
> + return -errno;
> +
> + while (bytes_written < max_len) {
> + len = write(fd, buf + bytes_written, max_len - bytes_written);
> +
> + if (len < 0)
> + break;
> + bytes_written += len;
> + }
> +
> + err = -errno;
> + close(fd);
> + return len < 0 ? err : bytes_written;
> +}
> +
> +static long read_num(const char *path)
> +{
> + char buf[21];
> +
> + if (read_text(path, buf, sizeof(buf)) <= 0)
> + return -1;
> + return (long)strtoul(buf, NULL, 10);
> +}
> +
> +static int write_num(const char *path, unsigned long n)
> +{
> + char buf[21];
> +
> + sprintf(buf, "%lu", n);
> + if (write_text(path, buf, strlen(buf)) < 0)
> + return -1;
> + return 0;
> +}
> +
> +long sysfs_get_refresh_interval(int nid)
> +{
> + char file[128];
> +
> + snprintf(
> + file,
> + sizeof(file),
> + 
> "/sys/devices/system/node/node%d/workingset_report/refresh_interval",
> + nid);
> + return read_num(file);
> +}
> +
> +int sysfs_set_refresh_interval(int nid, long interval)
> 

[PATCH] kunit: tool: Build compile_commands.json

2024-05-16 Thread Brendan Jackman
compile_commands.json is used by clangd[1] to provide code navigation
and completion functionality to editors. See [2] for an example
configuration that includes this functionality for VSCode.

It can currently be built manually when using kunit.py, by running:

  ./scripts/clang-tools/gen_compile_commands.py -d .kunit

With this change however, it's built automatically so you don't need to
manually keep it up to date.

Unlike the manual approach, having make build the compile_commands.json
means that it appears in the build output tree instead of at the root of
the source tree, so you'll need to add --compile-commands-dir=.kunit to
your clangd args for it to be found. This might turn out to be pretty
annoying, I'm not sure yet. If so maybe we can later add some hackery to
kunit.py to work around it.

[1] https://clangd.llvm.org/
[2] https://github.com/FlorentRevest/linux-kernel-vscode

Signed-off-by: Brendan Jackman 
---
 tools/testing/kunit/kunit_kernel.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/kunit/kunit_kernel.py 
b/tools/testing/kunit/kunit_kernel.py
index 7254c110ff23..61931c4926fd 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -72,7 +72,8 @@ class LinuxSourceTreeOperations:
raise ConfigError(e.output.decode())
 
def make(self, jobs: int, build_dir: str, make_options: 
Optional[List[str]]) -> None:
-   command = ['make', 'ARCH=' + self._linux_arch, 'O=' + 
build_dir, '--jobs=' + str(jobs)]
+   command = ['make', 'all', 'compile_commands.json', 'ARCH=' + 
self._linux_arch,
+  'O=' + build_dir, '--jobs=' + str(jobs)]
if make_options:
command.extend(make_options)
if self._cross_compile:

---
base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
change-id: 20240516-kunit-compile-commands-d994074fc2be

Best regards,
-- 
Brendan Jackman 




[PATCH] kunit: tool: Build compile_commands.json

2024-05-16 Thread Brendan Jackman
compile_commands.json is used by clangd[1] to provide code navigation
and completion functionality to editors. See [2] for an example
configuration that includes this functionality for VSCode.

It can currently be built manually when using kunit.py, by running:

  ./scripts/clang-tools/gen_compile_commands.py -d .kunit

With this change however, it's built automatically so you don't need to
manually keep it up to date.

Unlike the manual approach, having make build the compile_commands.json
means that it appears in the build output tree instead of at the root of
the source tree, so you'll need to add --compile-commands-dir= to your
clangd args for it to be found.

[1] https://clangd.llvm.org/
[2] https://github.com/FlorentRevest/linux-kernel-vscode

Signed-off-by: Brendan Jackman 
---
 tools/testing/kunit/kunit_kernel.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/kunit/kunit_kernel.py 
b/tools/testing/kunit/kunit_kernel.py
index 7254c110ff23..61931c4926fd 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -72,7 +72,8 @@ class LinuxSourceTreeOperations:
raise ConfigError(e.output.decode())
 
def make(self, jobs: int, build_dir: str, make_options: 
Optional[List[str]]) -> None:
-   command = ['make', 'ARCH=' + self._linux_arch, 'O=' + 
build_dir, '--jobs=' + str(jobs)]
+   command = ['make', 'all', 'compile_commands.json', 'ARCH=' + 
self._linux_arch,
+  'O=' + build_dir, '--jobs=' + str(jobs)]
if make_options:
command.extend(make_options)
if self._cross_compile:

---
base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
change-id: 20240516-kunit-compile-commands-d994074fc2be

Best regards,
-- 
Brendan Jackman 




Re: [PATCH v4] kunit: Cover 'assert.c' with tests

2024-05-16 Thread Rae Moar
On Wed, May 15, 2024 at 10:20 AM Ivan Orlov  wrote:
>
> There are multiple assertion formatting functions in the `assert.c`
> file, which are not covered with tests yet. Implement the KUnit test
> for these functions.
>
> The test consists of 11 test cases for the following functions:
>
> 1) 'is_literal'
> 2) 'is_str_literal'
> 3) 'kunit_assert_prologue', test case for multiple assert types
> 4) 'kunit_assert_print_msg'
> 5) 'kunit_unary_assert_format'
> 6) 'kunit_ptr_not_err_assert_format'
> 7) 'kunit_binary_assert_format'
> 8) 'kunit_binary_ptr_assert_format'
> 9) 'kunit_binary_str_assert_format'
> 10) 'kunit_assert_hexdump'
> 11) 'kunit_mem_assert_format'
>
> The test aims at maximizing the branch coverage for the assertion
> formatting functions.
>
> As you can see, it covers some of the static helper functions as
> well, so mark the static functions in `assert.c` as 'VISIBLE_IF_KUNIT'
> and conditionally export them with EXPORT_SYMBOL_IF_KUNIT. Add the
> corresponding definitions to `assert.h`.
>
> Build the assert test when CONFIG_KUNIT_TEST is enabled, similar to
> how it is done for the string stream test.
>
> Signed-off-by: Ivan Orlov 
> ---
> V1 -> V2:
> - Check the output from the string stream for containing the key parts
> instead of comparing the results with expected strings char by char, as
> it was suggested by Rae Moar . Define two macros to
> make it possible (ASSERT_TEST_EXPECT_CONTAIN and
> ASSERT_TEST_EXPECT_NCONTAIN).
> - Mark the static functions in `assert.c` as VISIBLE_IF_KUNIT and export
> them conditionally if kunit is enabled instead of including the
> `assert_test.c` file in the end of `assert.c`. This way we will decouple
> the test from the implementation (SUT).
> - Update the kunit_assert_hexdump test: now it checks for presense of
> the brackets '<>' around the non-matching bytes, instead of comparing
> the kunit_assert_hexdump output char by char.
> V2 -> V3:
> - Make test case array and test suite definitions static
> - Change the condition in `assert.h`: we should declare VISIBLE_IF_KUNIT
> functions in the header file when CONFIG_KUNIT is enabled, not
> CONFIG_KUNIT_TEST. Otherwise, if CONFIG_KUNIT_TEST is disabled,
> VISIBLE_IF_KUNIT functions in the `assert.c` are not static, and
> prototypes for them can't be found.
> - Add MODULE_LICENSE and MODULE_DESCRIPTION macros
> V3 -> V4:
> - Compile the assertion test only when CONFIG_KUNIT_TEST is set to 'y',
> as it is done for the string-stream test. It is necessary since
> functions from the string-stream are not exported into the public
> namespace, and therefore they are not accessible when linking and
> loading the test module.

Hi Ivan!

This looks great! Just one last thing, since this test is no longer
loadable as a module, could you remove the exporting of new symbols
and adding of the module license. Those can be part of the next patch,
where we convert these tests to modules.

Thanks!
-Rae

>
>  include/kunit/assert.h  |  11 ++
>  lib/kunit/Makefile  |   1 +
>  lib/kunit/assert.c  |  24 ++-
>  lib/kunit/assert_test.c | 391 
>  4 files changed, 419 insertions(+), 8 deletions(-)
>  create mode 100644 lib/kunit/assert_test.c
>
> diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> index 24c2b9fa61e8..7e7490a74b13 100644
> --- a/include/kunit/assert.h
> +++ b/include/kunit/assert.h
> @@ -218,4 +218,15 @@ void kunit_mem_assert_format(const struct kunit_assert 
> *assert,
>  const struct va_format *message,
>  struct string_stream *stream);
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +void kunit_assert_print_msg(const struct va_format *message,
> +   struct string_stream *stream);
> +bool is_literal(const char *text, long long value);
> +bool is_str_literal(const char *text, const char *value);
> +void kunit_assert_hexdump(struct string_stream *stream,
> + const void *buf,
> + const void *compared_buf,
> + const size_t len);
> +#endif
> +
>  #endif /*  _KUNIT_ASSERT_H */
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 309659a32a78..900e6447c8e8 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_KUNIT_TEST) +=   kunit-test.o
>  # string-stream-test compiles built-in only.
>  ifeq ($(CONFIG_KUNIT_TEST),y)
>  obj-$(CONFIG_KUNIT_TEST) +=string-stream-test.o
> +obj-$(CONFIG_KUNIT_TEST) +=assert_test.o
>  endif
>
>  obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=kunit-example-test.o
> diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
> index dd1d633d0fe2..382eb409d34b 100644
> --- a/lib/kunit/assert.c
> +++ b/lib/kunit/assert.c
> @@ -7,6 +7,7 @@
>   */
>  #include 
>  #include 
> +#include 
>
>  #include "string-stream.h"
>
> @@ -30,12 +31,14 @@ void kunit_assert_prologue(const struct kunit_loc *loc,
>  }
>  

Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Shuah Khan

On 5/16/24 11:45, Tejun Heo wrote:

Hello,

On Thu, May 16, 2024 at 10:31:13AM -0600, Shuah Khan wrote:

I am exploring options and leaning towards reverting the patch

daef47b89efd ("selftests: Compile kselftest headers with -D_GNU_SOURCE")

Your amending the PR helps me if I have to send revert. I am sorry
for the trouble.

I can all of them together in a second update or after the merge window
closes.


The cgroup commit is already pulled in unfortunately. Can you please handle
the revert and whatever's necessary to fix up the situation? I'll ask you
what to do with selftest patches from now on.



Thanks for the update. Yes I am working on fixing the situation and
will send revert for cgroup test patch as well if necessary.

No worries. It is not a problem for you to handle cgroup test patches
in general. I will need your review anyway and letting you handle them
reduces the overhead.

This kind of framework change causes needs to be coordinated.
I should have held back on the framework change on my part.

thanks,
-- Shuah




Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Tejun Heo
Hello,

On Thu, May 16, 2024 at 10:31:13AM -0600, Shuah Khan wrote:
> I am exploring options and leaning towards reverting the patch
> 
> daef47b89efd ("selftests: Compile kselftest headers with -D_GNU_SOURCE")
> 
> Your amending the PR helps me if I have to send revert. I am sorry
> for the trouble.
> 
> I can all of them together in a second update or after the merge window
> closes.

The cgroup commit is already pulled in unfortunately. Can you please handle
the revert and whatever's necessary to fix up the situation? I'll ask you
what to do with selftest patches from now on.

Thank you.

-- 
tejun



Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Shuah Khan

On 5/16/24 10:21, Tejun Heo wrote:

On Thu, May 16, 2024 at 09:50:06AM -0600, Shuah Khan wrote:

On 5/13/24 11:02, Tejun Heo wrote:

On Fri, May 10, 2024 at 12:06:25AM +, Edward Liaw wrote:

_GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
redefinition warnings.

Signed-off-by: Edward Liaw 


Applied to cgroup/for-6.10.

Thanks.



Hi Tejun,

Please don't include this in your PR to Linus. This patch series needs
to go together as it is causing several build warns and some errors.


I'm afraid it's too late. The PR is too late. Do you want me to send an
amended PR with the commit reverted? If it's just temporary issues in
selftests, maybe we can just wait it out?



I am exploring options and leaning towards reverting the patch

daef47b89efd ("selftests: Compile kselftest headers with -D_GNU_SOURCE")

Your amending the PR helps me if I have to send revert. I am sorry
for the trouble.

I can all of them together in a second update or after the merge window
closes.

thanks,
-- Shuah





Re: [PATCH v6 03/17] riscv: vector: Use vlenb from DT

2024-05-16 Thread Conor Dooley
On Thu, May 16, 2024 at 10:00:12PM +0800, Andy Chiu wrote:
> On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins  wrote:

> > +   if (elf_hwcap & COMPAT_HWCAP_ISA_V && 
> > has_riscv_homogeneous_vlenb() < 0) {
> > +   pr_warn("Unsupported heterogeneous vlen detected, 
> > vector extension disabled.\
> > +   elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> > +   }
> 
> We only touch COMPAT_HWCAP_ISA_V and the failed case only turns off the
> rectified V. So here we have nothing to do with the Xtheadvector.

There's nothing t-head related in the tree at this point, so doing
anything with it would cause build issues.

> However, I am still confused because I think Xtheadvector would also
> need to call into this check, so as to setup vlenb.


> Apart from that, it seems like some vendor stating Xtheadvector is
> actually vector-0.7.

The T-Head implementation is 0.7.x, but I am not really sure what you
mean by this comment.

> Please correct me if I speak anything wrong. One
> thing I noticed is that Xtheadvector wouldn't trap on reading
> th.vlenb but vector-0.7 would. If that is the case, should we require
> Xtheadvector to specify `riscv,vlenb` on the device tree?

In the world of Linux, "vector-0.7" isn't a thing. There's only 1.0, and
after this patchset, "xtheadvector". My understanding, from discussion
on earlier versions of this series the trap is actually accessing
th.vlenb register, despite the documentation stating that it is
unprivileged:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
I assume Charlie tried it but was trapping, as v1 had a comment:
+* Although xtheadvector states that th.vlenb exists and
+* overlaps with the vector 1.0 extension overlaps, an illegal
+* instruction is raised if read. These systems all currently
+* have a fixed vector length of 128, so hardcode that value.

Cheers,
Conor.


signature.asc
Description: PGP signature


Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Tejun Heo
On Thu, May 16, 2024 at 09:50:06AM -0600, Shuah Khan wrote:
> On 5/13/24 11:02, Tejun Heo wrote:
> > On Fri, May 10, 2024 at 12:06:25AM +, Edward Liaw wrote:
> > > _GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
> > > redefinition warnings.
> > > 
> > > Signed-off-by: Edward Liaw 
> > 
> > Applied to cgroup/for-6.10.
> > 
> > Thanks.
> > 
> 
> Hi Tejun,
> 
> Please don't include this in your PR to Linus. This patch series needs
> to go together as it is causing several build warns and some errors.

I'm afraid it's too late. The PR is too late. Do you want me to send an
amended PR with the commit reverted? If it's just temporary issues in
selftests, maybe we can just wait it out?

Thanks.

-- 
tejun



Re: [PATCH] kselftest/alsa: Ensure _GNU_SOURCE is defined

2024-05-16 Thread Muhammad Usama Anjum
On 5/16/24 9:27 AM, Mark Brown wrote:
> The pcmtest driver tests use the kselftest harness which requires that
> _GNU_SOURCE is defined but nothing causes it to be defined.  Since the
> KHDR_INCLUDES Makefile variable has had the required define added let's
> use that, this should provide some futureproofing.
> 
> Fixes: daef47b89efd ("selftests: Compile kselftest headers with 
> -D_GNU_SOURCE")
> Signed-off-by: Mark Brown 
Reviewed-by: Muhammad Usama Anjum 

> ---
>  tools/testing/selftests/alsa/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/alsa/Makefile 
> b/tools/testing/selftests/alsa/Makefile
> index 5af9ba8a4645..c1ce39874e2b 100644
> --- a/tools/testing/selftests/alsa/Makefile
> +++ b/tools/testing/selftests/alsa/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  #
>  
> -CFLAGS += $(shell pkg-config --cflags alsa)
> +CFLAGS += $(shell pkg-config --cflags alsa) $(KHDR_INCLUDES)
>  LDLIBS += $(shell pkg-config --libs alsa)
>  ifeq ($(LDLIBS),)
>  LDLIBS += -lasound
> 
> ---
> base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
> change-id: 20240516-kselftest-fix-gnu-source-81ddd00870a8
> 
> Best regards,

-- 
BR,
Muhammad Usama Anjum



Re: [PATCH v4 00/66] Define _GNU_SOURCE for sources using

2024-05-16 Thread Shuah Khan

On 5/9/24 18:06, Edward Liaw wrote:

Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes
redefinitions of _GNU_SOURCE from source code.

809216233555 ("selftests/harness: remove use of LINE_MAX") introduced
asprintf into kselftest_harness.h, which is a GNU extension and needs
_GNU_SOURCE to either be defined prior to including headers or with the
-D_GNU_SOURCE flag passed to the compiler.

v1: 
https://lore.kernel.org/linux-kselftest/20240430235057.1351993-1-edl...@google.com/
v2: 
https://lore.kernel.org/linux-kselftest/20240507214254.2787305-1-edl...@google.com/
  - Add -D_GNU_SOURCE to KHDR_INCLUDES so that it is in a single
location.
  - Remove #define _GNU_SOURCE from source code to resolve redefinition
warnings.
v3: 
https://lore.kernel.org/linux-kselftest/20240509200022.253089-1-edl...@google.com/
  - Rebase onto linux-next 20240508.
  - Split patches by directory.
  - Add -D_GNU_SOURCE directly to CFLAGS in lib.mk.
  - Delete additional _GNU_SOURCE definitions from source code in
linux-next.
  - Delete additional -D_GNU_SOURCE flags from Makefiles.
v4:
  - Rebase onto linux-next 20240509.
  - Remove Fixes tag from patches that drop _GNU_SOURCE definition.
  - Restore space between comment and includes for selftests/damon.
> Edward Liaw (66):
   selftests: Compile with -D_GNU_SOURCE when including lib.mk


This above change is causing some build problems - I didn't
notice them when I tested on linux-next. However some problems
are seen by Mark. He sent in a fix for ALSA and a change to
descalate build warn.

Please don't apply these for 6.10 for now. I will take all
of these together.

thanks,
-- Shuah



Re: [PATCH v4 08/66] selftests/cgroup: Drop define _GNU_SOURCE

2024-05-16 Thread Shuah Khan

On 5/13/24 11:02, Tejun Heo wrote:

On Fri, May 10, 2024 at 12:06:25AM +, Edward Liaw wrote:

_GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
redefinition warnings.

Signed-off-by: Edward Liaw 


Applied to cgroup/for-6.10.

Thanks.



Hi Tejun,

Please don't include this in your PR to Linus. This patch series needs
to go together as it is causing several build warns and some errors.

thanks,
-- Shuah



Re: [PATCH net] selftests/net: use tc rule to filter the na packet

2024-05-16 Thread Jakub Kicinski
On Tue, 14 May 2024 15:11:30 +0800 Hangbin Liu wrote:
> Hi Jakub, would you please help check if this fix the
> arp_ndisc_untracked_subnets flake issue on debug kernel?

It didn't get ingested by the CI because there's a conflict with
something else that got merged into lib.sh. Could you rebase / repost?


At a glance the problem in the CI is that it times out on debug kernels:

# overriding timeout to 7200
# selftests: net: arp_ndisc_untracked_subnets.sh
# TEST: test_arp:  accept_arp=0   [ OK ]
# TEST: test_arp:  accept_arp=1   [ OK ]
# TEST: test_arp:  accept_arp=2  same_subnet=0[ OK ]
# TEST: test_arp:  accept_arp=2  same_subnet=1[ OK ]
#
not ok 1 selftests: net: arp_ndisc_untracked_subnets.sh # TIMEOUT 7200 seconds

So it consumed full 2 hours and didn't finish.



[PATCH] kselftest: Desecalate reporting of missing _GNU_SOURCE

2024-05-16 Thread Mark Brown
Commit daef47b89efd0b7 ("selftests: Compile kselftest headers with
-D_GNU_SOURCE") adds a static_assert() which means that things which
would be warnings about undeclared functions get escalated into build
failures.  While we do actually want _GNU_SOURCE to be defined for users
of kselftest_harness we haven't actually done that yet and this is
causing widespread build breaks which were previously warnings about
uses of asprintf() without prototypes, including causing other test
programs in the same directory to fail to build.

Since the build failures that are introduced cause additional issues due
to make stopping builds early replace the static_assert() with a
missing without making the error more severe than it already was.  This
will be moot once the issue is fixed properly but reduces the disruption
while that happens.

Signed-off-by: Mark Brown 
---
 tools/testing/selftests/kselftest_harness.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest_harness.h 
b/tools/testing/selftests/kselftest_harness.h
index 37b03f1b8741..1cee8cacf9dc 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -51,7 +51,7 @@
 #define __KSELFTEST_HARNESS_H
 
 #ifndef _GNU_SOURCE
-static_assert(0, "kselftest harness requires _GNU_SOURCE to be defined");
+#warning kselftest harness requires _GNU_SOURCE to be defined
 #endif
 #include 
 #include 

---
base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
change-id: 20240516-kselftest-mitigate-gnu-source-b41b2d2cb8a1

Best regards,
-- 
Mark Brown 




[PATCH] kselftest/alsa: Ensure _GNU_SOURCE is defined

2024-05-16 Thread Mark Brown
The pcmtest driver tests use the kselftest harness which requires that
_GNU_SOURCE is defined but nothing causes it to be defined.  Since the
KHDR_INCLUDES Makefile variable has had the required define added let's
use that, this should provide some futureproofing.

Fixes: daef47b89efd ("selftests: Compile kselftest headers with -D_GNU_SOURCE")
Signed-off-by: Mark Brown 
---
 tools/testing/selftests/alsa/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/alsa/Makefile 
b/tools/testing/selftests/alsa/Makefile
index 5af9ba8a4645..c1ce39874e2b 100644
--- a/tools/testing/selftests/alsa/Makefile
+++ b/tools/testing/selftests/alsa/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 #
 
-CFLAGS += $(shell pkg-config --cflags alsa)
+CFLAGS += $(shell pkg-config --cflags alsa) $(KHDR_INCLUDES)
 LDLIBS += $(shell pkg-config --libs alsa)
 ifeq ($(LDLIBS),)
 LDLIBS += -lasound

---
base-commit: 3c999d1ae3c75991902a1a7dad0cb62c2a3008b4
change-id: 20240516-kselftest-fix-gnu-source-81ddd00870a8

Best regards,
-- 
Mark Brown 




[PATCH net v3] selftests: net: local_termination: annotate the expected failures

2024-05-16 Thread Jakub Kicinski
Vladimir said when adding this test:

  The bridge driver fares particularly badly [...] mainly because
  it does not implement IFF_UNICAST_FLT.

See commit 90b9566aa5cd ("selftests: forwarding: add a test for
local_termination.sh").

We don't want to hide the known gaps, but having a test which
always fails prevents us from catching regressions. Report
the cases we know may fail as XFAIL.

Signed-off-by: Jakub Kicinski 
---
CC: liuhang...@gmail.com
CC: sh...@kernel.org
CC: linux-kselftest@vger.kernel.org
CC: Petr Machata 
CC: vladimir.olt...@nxp.com

v3:
 - use xfail_on_veth correctly as a "prefix" call
 - dropping Vladimir's tags since the code is quite different now
v2: https://lore.kernel.org/r/20240509235553.5740-1-k...@kernel.org/
 - remove duplicated log_test_xfail
v1: https://lore.kernel.org/all/20240509235553.5740-1-k...@kernel.org/
---
 .../net/forwarding/local_termination.sh   | 30 +++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh 
b/tools/testing/selftests/net/forwarding/local_termination.sh
index c5b0cbc85b3e..4b364cdf3ef0 100755
--- a/tools/testing/selftests/net/forwarding/local_termination.sh
+++ b/tools/testing/selftests/net/forwarding/local_termination.sh
@@ -155,25 +155,30 @@ run_test()
"$smac > $MACVLAN_ADDR, ethertype IPv4 (0x0800)" \
true
 
-   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
-   "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
-   false
+   xfail_on_veth $h1 \
+   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
+   "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
+   false
 
check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \
"$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \
true
 
-   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \
-   "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \
-   false
+   xfail_on_veth $h1 \
+   check_rcv $rcv_if_name \
+   "Unicast IPv4 to unknown MAC address, allmulti" \
+   "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \
+   false
 
check_rcv $rcv_if_name "Multicast IPv4 to joined group" \
"$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \
true
 
-   check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \
-   "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \
-   false
+   xfail_on_veth $h1 \
+   check_rcv $rcv_if_name \
+   "Multicast IPv4 to unknown group" \
+   "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 
(0x0800)" \
+   false
 
check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \
"$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \
@@ -187,9 +192,10 @@ run_test()
"$smac > $JOINED_MACV6_MC_ADDR, ethertype IPv6 (0x86dd)" \
true
 
-   check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \
-   "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \
-   false
+   xfail_on_veth $h1 \
+   check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \
+   "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 
(0x86dd)" \
+   false
 
check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \
"$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \
-- 
2.45.0




Re: [PATCH v6] lib: add basic KUnit test for lib/math

2024-05-16 Thread Daniel Latypov
On Thu, May 16, 2024 at 3:19 AM Devarsh Thakkar  wrote:
>
> Hi Daniel, Andy,
>
> On 16/04/21 23:34, Daniel Latypov wrote:
> > Add basic test coverage for files that don't require any config options:
> > * part of math.h (what seem to be the most commonly used macros)
> > * gcd.c
> > * lcm.c
> > * int_sqrt.c
> > * reciprocal_div.c
> > (Ignored int_pow.c since it's a simple textbook algorithm.)
> >
> > These tests aren't particularly interesting, but they
> > * provide short and simple examples of parameterized tests
> > * provide a place to add tests for any new files in this dir
> > * are written so adding new test cases to cover edge cases should be easy
> >   * looking at code coverage, we hit all the branches in the .c files
> >
> > Signed-off-by: Daniel Latypov 
> > Reviewed-by: David Gow 
>
> Just checking if something else was pending on this patch-set for this not
> getting merged?
>
> I needed this patch-set for adding tests for new macros I am adding in math.h
> as suggested in this thread [1], so wanted to pull this in my series and add
> changes on top of that for new macros.
>
> Kindly let me know your thoughts on this.

This patch just fell through the cracks for me.
I had (wrongly) inferred that Andy might have had some lingering
reservations about this patch (that it was too contrived, might not be
useful to have tests for stuff like abs(), etc.).

Feel free to pull this into your series.

Looking over the code itself, I think this still looks valid and
stylistically correct with regard to KUnit.
I haven't gone and validated that it still compiles and runs just yet, though.
But if you do run into any problems, let me know and I can help send
you a fixed version.

Thanks for picking this up,
Daniel



Re: -D_GNU_SOURCE kselftest breakage in mainline

2024-05-16 Thread Mark Brown
On Thu, May 16, 2024 at 08:53:52AM -0600, Shuah Khan wrote:
> On 5/16/24 08:02, Mark Brown wrote:

> > I'm seeing quite a lot of breakage in mainline as a result of
> > daef47b89efd0b7 ("selftests: Compile kselftest headers with
> > -D_GNU_SOURCE") and daef47b89efd0 ("selftests: Compile kselftest headers
> > with -D_GNU_SOURCE") - thus far I've found that the use of
> > static_assert() is triggering build breaks where testsuites aren't
> > picking up the addition of _GNU_SOURCE (including stopping installing
> > the other tests in the same directory), and there's a bunch of tests
> > which #define _GNU_SOURCE in their code and now trigger build warnings.
> > I'm looking at fixes and mitigations now.

> Would it be better to revert this for now and get this for now? I wouldn't
> want you to extra busy work to workaround this.

I have patches that both fix the build breakage (it turns out on closer
inspection that it's just ALSA that's impacted I think, the extensive
extra warning splat and some other stuff that's just updates in
mainline) was making the impact look wider) and deescalate it to a
#warning which I'm currently running through CI.

> > The build failures are taking out the ALSA tests entirely which has
> > caused my personal CI to explode badly :/

> This has been in next for a while and I didn't see any failures. These
> kind of changes are tricky. On second thought, I probably should have
> delayed picking this up.

Yeah, I'd been on holiday for literally a month (got home Monday night)
- once I saw the issues I realised I had registered them going past at
some point while I was away but hadn't really been paying attention to
what was going on in -next.  I'm kind of surprised nobody else picked up
on the warnings though.


signature.asc
Description: PGP signature


Re: -D_GNU_SOURCE kselftest breakage in mainline

2024-05-16 Thread Shuah Khan

On 5/16/24 08:02, Mark Brown wrote:

Hi,

I'm seeing quite a lot of breakage in mainline as a result of
daef47b89efd0b7 ("selftests: Compile kselftest headers with
-D_GNU_SOURCE") and daef47b89efd0 ("selftests: Compile kselftest headers
with -D_GNU_SOURCE") - thus far I've found that the use of
static_assert() is triggering build breaks where testsuites aren't
picking up the addition of _GNU_SOURCE (including stopping installing
the other tests in the same directory), and there's a bunch of tests
which #define _GNU_SOURCE in their code and now trigger build warnings.
I'm looking at fixes and mitigations now.



Would it be better to revert this for now and get this for now? I wouldn't
want you to extra busy work to workaround this.


The build failures are taking out the ALSA tests entirely which has
caused my personal CI to explode badly :/



This has been in next for a while and I didn't see any failures. These
kind of changes are tricky. On second thought, I probably should have
delayed picking this up.

thanks,
-- Shuah





Re: [PATCH 0/8] selftests: x86: build suite with clang

2024-05-16 Thread Muhammad Usama Anjum
On 5/1/24 6:29 AM, Muhammad Usama Anjum wrote:
> This series fixes build errors found by clang to allow the x86 suite to
> get built with the clang.
> 
> Unfortunately, there is one bug [1] in the clang becuase of which
> extended asm isn't handled correctly by it and build fails for
> sysret_rip.c. Hence even after this series the build of this test would
> fail with clang. Should we disable this test for now when clang is used
> until the bug is fixed in clang? Not sure. Any opinions?
Its seems like the bug has been fixed in clang. I'll verify it in next
release and draft a patch separately to fix that error.

I think this series is good to accept then.

> 
> [1] https://github.com/llvm/llvm-project/issues/53728
> 
> Muhammad Usama Anjum (8):
>   selftests: x86: Remove dependence of headers file
>   selftests: x86: check_initial_reg_state: remove -no-pie while using
> -static
>   selftests: x86: test_vsyscall: remove unused function
>   selftests: x86: fsgsbase_restore: fix asm directive from =rm to =r
>   selftests: x86: syscall_arg_fault_32: remove unused variable
>   selftests: x86: test_FISTTP: use fisttps instead of ambigous fisttp
>   selftests: x86: fsgsbase: Remove unused function and variable
>   selftests: x86: amx: Remove unused functions
> 
>  tools/testing/selftests/x86/Makefile|  9 +
>  tools/testing/selftests/x86/amx.c   | 16 
>  tools/testing/selftests/x86/fsgsbase.c  |  6 --
>  tools/testing/selftests/x86/fsgsbase_restore.c  |  2 +-
>  tools/testing/selftests/x86/syscall_arg_fault.c |  1 -
>  tools/testing/selftests/x86/test_FISTTP.c   |  8 
>  tools/testing/selftests/x86/test_vsyscall.c |  5 -
>  7 files changed, 10 insertions(+), 37 deletions(-)
> 

-- 
BR,
Muhammad Usama Anjum



-D_GNU_SOURCE kselftest breakage in mainline

2024-05-16 Thread Mark Brown
Hi,

I'm seeing quite a lot of breakage in mainline as a result of
daef47b89efd0b7 ("selftests: Compile kselftest headers with
-D_GNU_SOURCE") and daef47b89efd0 ("selftests: Compile kselftest headers
with -D_GNU_SOURCE") - thus far I've found that the use of
static_assert() is triggering build breaks where testsuites aren't
picking up the addition of _GNU_SOURCE (including stopping installing
the other tests in the same directory), and there's a bunch of tests
which #define _GNU_SOURCE in their code and now trigger build warnings.
I'm looking at fixes and mitigations now.

The build failures are taking out the ALSA tests entirely which has
caused my personal CI to explode badly :/

Thanks,
Mark


signature.asc
Description: PGP signature


Re: [PATCH v6 03/17] riscv: vector: Use vlenb from DT

2024-05-16 Thread Andy Chiu
Sorry Charlie, I forgot to include the mailing list. Here is the same as
what I sent in the private message.

On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins  wrote:
>
> If vlenb is provided in the device tree, prefer that over reading the
> vlenb csr.
>
> Signed-off-by: Charlie Jenkins 
> ---
>  arch/riscv/include/asm/cpufeature.h |  2 ++
>  arch/riscv/kernel/cpufeature.c  | 47 
> +
>  arch/riscv/kernel/vector.c  | 12 +-
>  3 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h 
> b/arch/riscv/include/asm/cpufeature.h
> index 347805446151..0c4f08577015 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
>  /* Per-cpu ISA extensions. */
>  extern struct riscv_isainfo hart_isa[NR_CPUS];
>
> +extern u32 riscv_vlenb_of;
> +
>  void riscv_user_isa_enable(void);
>
>  #if defined(CONFIG_RISCV_MISALIGNED)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..6c143ea9592b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) 
> __read_mostly;
>  /* Per-cpu ISA extensions. */
>  struct riscv_isainfo hart_isa[NR_CPUS];
>
> +u32 riscv_vlenb_of;
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -648,6 +650,46 @@ static int __init riscv_isa_fallback_setup(char 
> *__unused)
>  early_param("riscv_isa_fallback", riscv_isa_fallback_setup);
>  #endif
>
> +static int has_riscv_homogeneous_vlenb(void)
> +{
> +   int cpu;
> +   u32 prev_vlenb = 0;
> +   u32 vlenb;
> +
> +   /* Ignore vlenb if vector is not enabled in the kernel */
> +   if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> +   return 0;
> +
> +   for_each_possible_cpu(cpu) {
> +   struct device_node *cpu_node;
> +
> +   cpu_node = of_cpu_device_node_get(cpu);
> +   if (!cpu_node) {
> +   pr_warn("Unable to find cpu node\n");
> +   return -ENOENT;
> +   }
> +
> +   if (of_property_read_u32(cpu_node, "riscv,vlenb", )) {
> +   of_node_put(cpu_node);
> +
> +   if (prev_vlenb)
> +   return -ENOENT;
> +   continue;
> +   }
> +
> +   if (prev_vlenb && vlenb != prev_vlenb) {
> +   of_node_put(cpu_node);
> +   return -ENOENT;
> +   }
> +
> +   prev_vlenb = vlenb;
> +   of_node_put(cpu_node);
> +   }
> +
> +   riscv_vlenb_of = vlenb;
> +   return 0;
> +}
> +
>  void __init riscv_fill_hwcap(void)
>  {
> char print_str[NUM_ALPHA_EXTS + 1];
> @@ -671,6 +713,11 @@ void __init riscv_fill_hwcap(void)
> pr_info("Falling back to deprecated \"riscv,isa\"\n");
> riscv_fill_hwcap_from_isa_string(isa2hwcap);
> }
> +
> +   if (elf_hwcap & COMPAT_HWCAP_ISA_V && 
> has_riscv_homogeneous_vlenb() < 0) {
> +   pr_warn("Unsupported heterogeneous vlen detected, 
> vector extension disabled.\> +   elf_hwcap &= 
> ~COMPAT_HWCAP_ISA_V;
> +   }

We only touch COMPAT_HWCAP_ISA_V and the failed case only turns off the
rectified V. So here we have nothing to do with the Xtheadvector.

However, I am still confused because I think Xtheadvector would also
need to call into this check, so as to setup vlenb.

Apart from that, it seems like some vendor stating Xtheadvector is
actually vector-0.7. Please correct me if I speak anything wrong. One
thing I noticed is that Xtheadvector wouldn't trap on reading
th.vlenb but vector-0.7 would. If that is the case, should we require
Xtheadvector to specify `riscv,vlenb` on the device tree?

> }
>
> /*
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 6727d1d3b8f2..e04586cdb7f0 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -33,7 +33,17 @@ int riscv_v_setup_vsize(void)
>  {
> unsigned long this_vsize;
>
> -   /* There are 32 vector registers with vlenb length. */
> +   /*
> +* There are 32 vector registers with vlenb length.
> +*
> +* If the riscv,vlenb property was provided by the firmware, use that
> +* instead of probing the CSRs.
> +*/
> +   if (riscv_vlenb_of) {
> +   this_vsize = riscv_vlenb_of * 32;
> +   return 0;
> +   }
> +
> riscv_v_enable();
> this_vsize = csr_read(CSR_VLENB) * 32;
> riscv_v_disable();
>
> --
> 2.44.0
>
>
> ___
> linux-riscv mailing list
> 

Re: [PATCH v6 03/17] riscv: vector: Use vlenb from DT

2024-05-16 Thread Andy Chiu
On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins  wrote:
>
> If vlenb is provided in the device tree, prefer that over reading the
> vlenb csr.
>
> Signed-off-by: Charlie Jenkins 

I agree with Conor that we need a mechanism to turn off v and all
depending extensions with has_riscv_homogeneous_vlenb(). And that can
come after this.

Thanks for adding the homogeneous vlen checking!

Reviewed-by: Andy Chiu 

> ---
>  arch/riscv/include/asm/cpufeature.h |  2 ++
>  arch/riscv/kernel/cpufeature.c  | 47 
> +
>  arch/riscv/kernel/vector.c  | 12 +-
>  3 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h 
> b/arch/riscv/include/asm/cpufeature.h
> index 347805446151..0c4f08577015 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
>  /* Per-cpu ISA extensions. */
>  extern struct riscv_isainfo hart_isa[NR_CPUS];
>
> +extern u32 riscv_vlenb_of;
> +
>  void riscv_user_isa_enable(void);
>
>  #if defined(CONFIG_RISCV_MISALIGNED)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..6c143ea9592b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) 
> __read_mostly;
>  /* Per-cpu ISA extensions. */
>  struct riscv_isainfo hart_isa[NR_CPUS];
>
> +u32 riscv_vlenb_of;
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -648,6 +650,46 @@ static int __init riscv_isa_fallback_setup(char 
> *__unused)
>  early_param("riscv_isa_fallback", riscv_isa_fallback_setup);
>  #endif
>
> +static int has_riscv_homogeneous_vlenb(void)
> +{
> +   int cpu;
> +   u32 prev_vlenb = 0;
> +   u32 vlenb;
> +
> +   /* Ignore vlenb if vector is not enabled in the kernel */
> +   if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> +   return 0;
> +
> +   for_each_possible_cpu(cpu) {
> +   struct device_node *cpu_node;
> +
> +   cpu_node = of_cpu_device_node_get(cpu);
> +   if (!cpu_node) {
> +   pr_warn("Unable to find cpu node\n");
> +   return -ENOENT;
> +   }
> +
> +   if (of_property_read_u32(cpu_node, "riscv,vlenb", )) {
> +   of_node_put(cpu_node);
> +
> +   if (prev_vlenb)
> +   return -ENOENT;
> +   continue;
> +   }
> +
> +   if (prev_vlenb && vlenb != prev_vlenb) {
> +   of_node_put(cpu_node);
> +   return -ENOENT;
> +   }
> +
> +   prev_vlenb = vlenb;
> +   of_node_put(cpu_node);
> +   }
> +
> +   riscv_vlenb_of = vlenb;
> +   return 0;
> +}
> +
>  void __init riscv_fill_hwcap(void)
>  {
> char print_str[NUM_ALPHA_EXTS + 1];
> @@ -671,6 +713,11 @@ void __init riscv_fill_hwcap(void)
> pr_info("Falling back to deprecated \"riscv,isa\"\n");
> riscv_fill_hwcap_from_isa_string(isa2hwcap);
> }
> +
> +   if (elf_hwcap & COMPAT_HWCAP_ISA_V && 
> has_riscv_homogeneous_vlenb() < 0) {
> +   pr_warn("Unsupported heterogeneous vlen detected, 
> vector extension disabled.\n");
> +   elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> +   }
> }
>
> /*
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 6727d1d3b8f2..e04586cdb7f0 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -33,7 +33,17 @@ int riscv_v_setup_vsize(void)
>  {
> unsigned long this_vsize;
>
> -   /* There are 32 vector registers with vlenb length. */
> +   /*
> +* There are 32 vector registers with vlenb length.
> +*
> +* If the riscv,vlenb property was provided by the firmware, use that
> +* instead of probing the CSRs.
> +*/
> +   if (riscv_vlenb_of) {
> +   this_vsize = riscv_vlenb_of * 32;
> +   return 0;
> +   }
> +
> riscv_v_enable();
> this_vsize = csr_read(CSR_VLENB) * 32;
> riscv_v_disable();
>
> --
> 2.44.0
>
>
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



Re: [PATCH v6 02/17] dt-bindings: riscv: cpus: add a vlen register length property

2024-05-16 Thread Andy Chiu
On Sat, May 4, 2024 at 3:33 AM Charlie Jenkins  wrote:
>
> From: Conor Dooley 
>
> Add a property analogous to the vlenb CSR so that software can detect
> the vector length of each CPU prior to it being brought online.
> Currently software has to assume that the vector length read from the
> boot CPU applies to all possible CPUs. On T-Head CPUs implementing
> pre-ratification vector, reading the th.vlenb CSR may produce an illegal
> instruction trap, so this property is required on such systems.
>
> Signed-off-by: Conor Dooley 
> Signed-off-by: Charlie Jenkins 

Reviewed-by: Andy Chiu 

> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml 
> b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index d87dd50f1a4b..edcb6a7d9319 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -94,6 +94,12 @@ properties:
>  description:
>The blocksize in bytes for the Zicboz cache operations.
>
> +  riscv,vlenb:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description:
> +  VLEN/8, the vector register length in bytes. This property is required 
> in
> +  systems where the vector register length is not identical on all harts.
> +
># RISC-V has multiple properties for cache op block sizes as the sizes
># differ between individual CBO extensions
>cache-op-block-size: false
>
> --
> 2.44.0
>
>
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



Re: [PATCH v6 01/17] dt-bindings: riscv: Add xtheadvector ISA extension description

2024-05-16 Thread Andy Chiu
On Sat, May 4, 2024 at 3:33 AM Charlie Jenkins  wrote:
>
> The xtheadvector ISA extension is described on the T-Head extension spec
> Github page [1] at commit 95358cb2cca9.
>
> Link: 
> https://github.com/T-head-Semi/thead-extension-spec/blob/95358cb2cca9489361c61d335e03d3134b14133f/xtheadvector.adoc
>  [1]
>
> Signed-off-by: Charlie Jenkins 
> Reviewed-by: Conor Dooley 

Reviewed-by: Andy Chiu 

> ---
>  Documentation/devicetree/bindings/riscv/extensions.yaml | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml 
> b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..99d2a9e8c52d 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -477,6 +477,10 @@ properties:
>  latency, as ratified in commit 56ed795 ("Update
>  riscv-crypto-spec-vector.adoc") of riscv-crypto.
>
> +# vendor extensions, each extension sorted alphanumerically under the
> +# vendor they belong to. Vendors are sorted alphanumerically as well.
> +
> +# Andes
>  - const: xandespmu
>description:
>  The Andes Technology performance monitor extension for counter 
> overflow
> @@ -484,5 +488,11 @@ properties:
>  Registers in the AX45MP datasheet.
>  
> https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
>
> +# T-HEAD
> +- const: xtheadvector
> +  description:
> +The T-HEAD specific 0.7.1 vector implementation as written in
> +
> https://github.com/T-head-Semi/thead-extension-spec/blob/95358cb2cca9489361c61d335e03d3134b14133f/xtheadvector.adoc.
> +
>  additionalProperties: true
>  ...
>
> --
> 2.44.0
>
>
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



Re: [PATCH net-next] selftests: net: local_termination: annotate the expected failures

2024-05-16 Thread Petr Machata


Jakub Kicinski  writes:

> On Wed, 15 May 2024 11:02:28 +0200 Petr Machata wrote:
>> >> And then either replace the existing xfail_on_veth's (there are just a
>> >> handful) or convert xfail_on_veth to a wrapper around xfail_on_kind.  
>> >
>> > I think the bridge thing we can workaround by just checking
>> > if ${NETIFS[p1]} is veth, rather than $rcv_if_name.
>> > Since the two behave the same.  
>> 
>> I don't follow. The test has two legs, one creates a VRF and attaches
>> p2, the other creates a bridge and attaches p2. Whether p1 and p2 are
>> veth or HW seems orthogonal to whether $rcv_if_name is a bridge or a
>> veth.
>
> Right, my superficial understanding was that the main distinction is
> whether p2/h2 can do the filtering (or possibly some offload happens).
> So if p1,p2 are veths we know to XFAIL, doesn't matter if we're in 
> the vrf or bridge configuration, cause these construct will not filter
> either.
>
> If I'm not making sense - I'm probably confused, I can code up what you
> suggested, it will work, just more LoC :)

I'm not sure myself, but from the commit message it looks like the issue
is with $rcv_if_name being the bridge.

But the patch that you inline is R-b'd and T-b'd by Vladimir, so I'm
going to assume it's doing the right thing.

> +# Clear internal failure tracking for the next test case
> +begin_test()
> +{
> +RET=0
> +FAIL_TO_XFAIL=
> +}
> +
>  check_err()
>  {
>   local err=$1
> diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh 
> b/tools/testing/selftests/net/forwarding/local_termination.sh
> index c5b0cbc85b3e..a241acc02498 100755
> --- a/tools/testing/selftests/net/forwarding/local_termination.sh
> +++ b/tools/testing/selftests/net/forwarding/local_termination.sh
> @@ -73,9 +73,12 @@ check_rcv()
>   local pattern=$3
>   local should_receive=$4
>   local should_fail=
> + local xfail_sw=$5
>  
>   [ $should_receive = true ] && should_fail=0 || should_fail=1
> - RET=0
> + begin_test
> + # check if main interface is veth
> + [ "$xfail_sw" == true ] && xfail_on_veth $h1

If xfail_on_veth $h1 is all that's needed, then I really don't see a
reason why not just do this:

check_rcv $rcv_if_name "Unicast IPv4 to primary MAC address" \
"$smac > $rcv_dmac, ethertype IPv4 (0x0800)" \
true

check_rcv $rcv_if_name "Unicast IPv4 to macvlan MAC address" \
"$smac > $MACVLAN_ADDR, ethertype IPv4 (0x0800)" \
true

xfail_on_veth $h1 \
check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
"$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
false

This should work now, in much the same way as this patch, but the intent
is IMHO clearer (vs. passing a mystery true), and FAIL_TO_XFAIL is
cleanly scoped and doesn't run the risk of leaking out of the test.

>   tcpdump_show $if_name | grep -q "$pattern"
>  
> @@ -157,7 +160,7 @@ run_test()
>  
>   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \
>   "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \
> - false
> + false true
>  
>   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \
>   "$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \
> @@ -165,7 +168,7 @@ run_test()
>  
>   check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \
>   "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \
> - false
> + false true
>  
>   check_rcv $rcv_if_name "Multicast IPv4 to joined group" \
>   "$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \
> @@ -173,7 +176,7 @@ run_test()
>  
>   check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \
>   "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \
> - false
> + false true
>  
>   check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \
>   "$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \
> @@ -189,7 +192,7 @@ run_test()
>  
>   check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \
>   "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \
> - false
> + false true
>  
>   check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \
>   "$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \




Re: [PATCH v6] lib: add basic KUnit test for lib/math

2024-05-16 Thread Andy Shevchenko
On Thu, May 16, 2024 at 03:49:44PM +0530, Devarsh Thakkar wrote:
> Hi Daniel, Andy,
> 
> On 16/04/21 23:34, Daniel Latypov wrote:
> > Add basic test coverage for files that don't require any config options:
> > * part of math.h (what seem to be the most commonly used macros)
> > * gcd.c
> > * lcm.c
> > * int_sqrt.c
> > * reciprocal_div.c
> > (Ignored int_pow.c since it's a simple textbook algorithm.)
> > 
> > These tests aren't particularly interesting, but they
> > * provide short and simple examples of parameterized tests
> > * provide a place to add tests for any new files in this dir
> > * are written so adding new test cases to cover edge cases should be easy
> >   * looking at code coverage, we hit all the branches in the .c files
> > 
> > Signed-off-by: Daniel Latypov 
> > Reviewed-by: David Gow 
> 
> Just checking if something else was pending on this patch-set for this not
> getting merged?
> 
> I needed this patch-set for adding tests for new macros I am adding in math.h
> as suggested in this thread [1], so wanted to pull this in my series and add
> changes on top of that for new macros.
> 
> Kindly let me know your thoughts on this.

Wow, blast from the past!
But good finding, it would be good to have more math.h related test cases.

-- 
With Best Regards,
Andy Shevchenko





Re: [PATCH v6] lib: add basic KUnit test for lib/math

2024-05-16 Thread Devarsh Thakkar
Hi Daniel, Andy,

On 16/04/21 23:34, Daniel Latypov wrote:
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
> 
> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy
>   * looking at code coverage, we hit all the branches in the .c files
> 
> Signed-off-by: Daniel Latypov 
> Reviewed-by: David Gow 

Just checking if something else was pending on this patch-set for this not
getting merged?

I needed this patch-set for adding tests for new macros I am adding in math.h
as suggested in this thread [1], so wanted to pull this in my series and add
changes on top of that for new macros.

Kindly let me know your thoughts on this.

[1]: https://lore.kernel.org/all/zkig0-01pz632...@smile.fi.intel.com/#t

Regards
Devarsh
> ---
> Changes since v5:
> * add in test cases for roundup/rounddown
> * address misc comments from David
> 
> Changes since v4:
> * add in test cases for some math.h macros (abs, round_up/round_down,
>   div_round_down/closest)
> * use parameterized testing less to keep things terser
> 
> Changes since v3:
> * fix `checkpatch.pl --strict` warnings
> * add test cases for gcd(0,0) and lcm(0,0)
> * minor: don't test both gcd(a,b) and gcd(b,a) when a == b
> 
> Changes since v2: mv math_test.c => math_kunit.c
> 
> Changes since v1:
> * Rebase and rewrite to use the new parameterized testing support.
> * misc: fix overflow in literal and inline int_sqrt format string.
> * related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
> for testing many inputs") was merged explaining the patterns shown here.
>   * there's an in-flight patch to update it for parameterized testing.
> ---
>  lib/math/Kconfig  |  12 ++
>  lib/math/Makefile |   2 +
>  lib/math/math_kunit.c | 291 ++
>  3 files changed, 305 insertions(+)
>  create mode 100644 lib/math/math_kunit.c
> 
> diff --git a/lib/math/Kconfig b/lib/math/Kconfig
> index f19bc9734fa7..a974d4db0f9c 100644
> --- a/lib/math/Kconfig
> +++ b/lib/math/Kconfig
> @@ -15,3 +15,15 @@ config PRIME_NUMBERS
>  
>  config RATIONAL
>   bool
> +
> +config MATH_KUNIT_TEST
> + tristate "KUnit test for lib/math and math.h" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds unit tests for lib/math and math.h.
> +
> + For more information on KUnit and unit tests in general, please 
> refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> diff --git a/lib/math/Makefile b/lib/math/Makefile
> index be6909e943bd..30abb7a8d564 100644
> --- a/lib/math/Makefile
> +++ b/lib/math/Makefile
> @@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o 
> reciprocal_div.o
>  obj-$(CONFIG_CORDIC) += cordic.o
>  obj-$(CONFIG_PRIME_NUMBERS)  += prime_numbers.o
>  obj-$(CONFIG_RATIONAL)   += rational.o
> +
> +obj-$(CONFIG_MATH_KUNIT_TEST)+= math_kunit.o
> diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c
> new file mode 100644
> index ..556c23b17c3c
> --- /dev/null
> +++ b/lib/math/math_kunit.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple KUnit suite for math helper funcs that are always enabled.
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Daniel Latypov 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static void abs_test(struct kunit *test)
> +{
> + KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
> + KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
> + KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
> +
> + /* The expression in the macro is actually promoted to an int. */
> + KUNIT_EXPECT_EQ(test, abs((short)0),  0);
> + KUNIT_EXPECT_EQ(test, abs((short)42),  42);
> + KUNIT_EXPECT_EQ(test, abs((short)-42),  42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0),  0);
> + KUNIT_EXPECT_EQ(test, abs(42),  42);
> + KUNIT_EXPECT_EQ(test, abs(-42),  42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0L), 0L);
> + KUNIT_EXPECT_EQ(test, abs(42L), 42L);
> + KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
> +
> + KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
> + KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
> + KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
> +
> + /* Unsigned types get casted to signed. */
> + KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
> + KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
> +}
> +
> +static void int_sqrt_test(struct kunit *test)
> +{
> + KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
> + KUNIT_EXPECT_EQ(test, 

[PATCH v6 8/8] selftests/pcie_bwctrl: Create selftests

2024-05-16 Thread Ilpo Järvinen
Create selftests for PCIe BW control through the PCIe cooling device
sysfs interface.

First, the BW control selftest finds the PCIe Port to test with. By
default, the PCIe Port with the highest Link Speed is selected but
another PCIe Port can be provided with -d parameter.

The actual test steps the cur_state of the cooling device one-by-one
from max_state to what the cur_state was initially. The speed change
is confirmed by observing the current_link_speed for the corresponding
PCIe Port.

Signed-off-by: Ilpo Järvinen 
---
 MAINTAINERS   |   1 +
 tools/testing/selftests/Makefile  |   1 +
 tools/testing/selftests/pcie_bwctrl/Makefile  |   2 +
 .../pcie_bwctrl/set_pcie_cooling_state.sh | 122 ++
 .../selftests/pcie_bwctrl/set_pcie_speed.sh   |  67 ++
 5 files changed, 193 insertions(+)
 create mode 100644 tools/testing/selftests/pcie_bwctrl/Makefile
 create mode 100755 
tools/testing/selftests/pcie_bwctrl/set_pcie_cooling_state.sh
 create mode 100755 tools/testing/selftests/pcie_bwctrl/set_pcie_speed.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 3a94ae81b13f..5a3b69515256 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17106,6 +17106,7 @@ S:  Supported
 F: drivers/pci/pcie/bwctrl.c
 F: drivers/thermal/pcie_cooling.c
 F: include/linux/pci-bwctrl.h
+F: tools/testing/selftests/pcie_bwctrl/
 
 PCIE DRIVER FOR AMAZON ANNAPURNA LABS
 M: Jonathan Chocron 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index e1504833654d..ac0bc8af4123 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -65,6 +65,7 @@ TARGETS += net/openvswitch
 TARGETS += net/tcp_ao
 TARGETS += netfilter
 TARGETS += nsfs
+TARGETS += pcie_bwctrl
 TARGETS += perf_events
 TARGETS += pidfd
 TARGETS += pid_namespace
diff --git a/tools/testing/selftests/pcie_bwctrl/Makefile 
b/tools/testing/selftests/pcie_bwctrl/Makefile
new file mode 100644
index ..3e84e26341d1
--- /dev/null
+++ b/tools/testing/selftests/pcie_bwctrl/Makefile
@@ -0,0 +1,2 @@
+TEST_PROGS = set_pcie_cooling_state.sh
+include ../lib.mk
diff --git a/tools/testing/selftests/pcie_bwctrl/set_pcie_cooling_state.sh 
b/tools/testing/selftests/pcie_bwctrl/set_pcie_cooling_state.sh
new file mode 100755
index ..9df606552af3
--- /dev/null
+++ b/tools/testing/selftests/pcie_bwctrl/set_pcie_cooling_state.sh
@@ -0,0 +1,122 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+SYSFS=
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+retval=0
+skipmsg="skip all tests:"
+
+PCIEPORTTYPE="PCIe_Port_Link_Speed"
+
+prerequisite()
+{
+   local ports
+
+   if [ $UID != 0 ]; then
+   echo $skipmsg must be run as root >&2
+   exit $ksft_skip
+   fi
+
+   SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
+
+   if [ ! -d "$SYSFS" ]; then
+   echo $skipmsg sysfs is not mounted >&2
+   exit $ksft_skip
+   fi
+
+   if ! ls $SYSFS/class/thermal/cooling_device* > /dev/null 2>&1; then
+   echo $skipmsg thermal cooling devices missing >&2
+   exit $ksft_skip
+   fi
+
+   ports=`grep -e "^$PCIEPORTTYPE" 
$SYSFS/class/thermal/cooling_device*/type | wc -l`
+   if [ $ports -eq 0 ]; then
+   echo $skipmsg pcie cooling devices missing >&2
+   exit $ksft_skip
+   fi
+}
+
+testport=
+find_pcie_port()
+{
+   local patt="$1"
+   local pcieports
+   local max
+   local cur
+   local delta
+   local bestdelta=-1
+
+   pcieports=`grep -l -F -e "$patt" 
/sys/class/thermal/cooling_device*/type`
+   if [ -z "$pcieports" ]; then
+   return
+   fi
+   pcieports=${pcieports//\/type/}
+   # Find the port with the highest PCIe Link Speed
+   for port in $pcieports; do
+   max=`cat $port/max_state`
+   cur=`cat $port/cur_state`
+   delta=$((max-cur))
+   if [ $delta -gt $bestdelta ]; then
+   testport="$port"
+   bestdelta=$delta
+   fi
+   done
+}
+
+sysfspcidev=
+find_sysfs_pci_dev()
+{
+   local typefile="$1/type"
+   local pcidir
+
+   pcidir="$SYSFS/bus/pci/devices/`sed -e "s|^${PCIEPORTTYPE}_||g" 
$typefile`"
+
+   if [ -r "$pcidir/current_link_speed" ]; then
+   sysfspcidev="$pcidir/current_link_speed"
+   fi
+}
+
+usage()
+{
+   echo "Usage $0 [ -d dev ]"
+   echo -e "\t-d: PCIe port BDF string (e.g., :00:04.0)"
+}
+
+pattern="$PCIEPORTTYPE"
+parse_arguments()
+{
+   while getopts d:h opt; do
+   case $opt in
+   h)
+   usage "$0"
+   exit 0
+   ;;
+   d)
+   pattern="$PCIEPORTTYPE_$OPTARG"
+ 

Re: [PATCH net] selftests/net/lib: no need to record ns name if it already exist

2024-05-16 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller :

On Tue, 14 May 2024 10:33:59 +0800 you wrote:
> There is no need to add the name to ns_list again if the netns already
> recoreded.
> 
> Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
> Signed-off-by: Hangbin Liu 
> ---
>  tools/testing/selftests/net/lib.sh | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [net] selftests/net/lib: no need to record ns name if it already exist
https://git.kernel.org/netdev/net/c/83e93942796d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html





Re: [PATCH v1] memfd: `MFD_NOEXEC_SEAL` should not imply `MFD_ALLOW_SEALING`

2024-05-16 Thread Jeff Xu
On Mon, May 13, 2024 at 12:15 PM Barnabás Pőcze  wrote:
>
> `MFD_NOEXEC_SEAL` should remove the executable bits and set
> `F_SEAL_EXEC` to prevent further modifications to the executable
> bits as per the comment in the uapi header file:
>
>   not executable and sealed to prevent changing to executable
>
> However, currently, it also unsets `F_SEAL_SEAL`, essentially
> acting as a superset of `MFD_ALLOW_SEALING`. Nothing implies
> that it should be so, and indeed up until the second version
> of the of the patchset[0] that introduced `MFD_EXEC` and
> `MFD_NOEXEC_SEAL`, `F_SEAL_SEAL` was not removed, however it
> was changed in the third revision of the patchset[1] without
> a clear explanation.
>
> This behaviour is suprising for application developers,
> there is no documentation that would reveal that `MFD_NOEXEC_SEAL`
> has the additional effect of `MFD_ALLOW_SEALING`.
>
Ya, I agree that there should be documentation, such as a man page. I will
work on that.

> So do not remove `F_SEAL_SEAL` when `MFD_NOEXEC_SEAL` is requested.
> This is technically an ABI break, but it seems very unlikely that an
> application would depend on this behaviour (unless by accident).
>
> [0]: https://lore.kernel.org/lkml/20220805222126.142525-3-jef...@google.com/
> [1]: https://lore.kernel.org/lkml/20221202013404.163143-3-jef...@google.com/
>
> Fixes: 105ff5339f498a ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC")
> Signed-off-by: Barnabás Pőcze 
> ---
>
> Or did I miss the explanation as to why MFD_NOEXEC_SEAL should
> imply MFD_ALLOW_SEALING? If so, please direct me to it and
> sorry for the noise.
>
Previously I might be thinking  MFD_NOEXEC_SEAL implies
MFD_ALLOW_SEALING because MFD_NOEXEC_SEAL seals F_SEAL_EXEC, and
sealing is added only when MFD_ALLOW_SEALING is set.

I agree your patch handles this better, e.g.
mfd_create(MFD_NOEXEC_SEAL) will have F_SEAL_SEAL and F_SEAL_EXEC
mfd_create(MFD_NOEXEC_SEAL|MFD_ALLOW_SEALING) will have F_SEAL_EXEC


> ---
>  mm/memfd.c | 9 -
>  tools/testing/selftests/memfd/memfd_test.c | 2 +-
>  2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 7d8d3ab3fa37..8b7f6afee21d 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -356,12 +356,11 @@ SYSCALL_DEFINE2(memfd_create,
>
> inode->i_mode &= ~0111;
> file_seals = memfd_file_seals_ptr(file);
> -   if (file_seals) {
> -   *file_seals &= ~F_SEAL_SEAL;
> +   if (file_seals)
> *file_seals |= F_SEAL_EXEC;
> -   }
> -   } else if (flags & MFD_ALLOW_SEALING) {
> -   /* MFD_EXEC and MFD_ALLOW_SEALING are set */
> +   }
> +
> +   if (flags & MFD_ALLOW_SEALING) {
> file_seals = memfd_file_seals_ptr(file);
> if (file_seals)
> *file_seals &= ~F_SEAL_SEAL;
> diff --git a/tools/testing/selftests/memfd/memfd_test.c 
> b/tools/testing/selftests/memfd/memfd_test.c
> index 18f585684e20..b6a7ad68c3c1 100644
> --- a/tools/testing/selftests/memfd/memfd_test.c
> +++ b/tools/testing/selftests/memfd/memfd_test.c
> @@ -1151,7 +1151,7 @@ static void test_noexec_seal(void)
> mfd_def_size,
> MFD_CLOEXEC | MFD_NOEXEC_SEAL);
> mfd_assert_mode(fd, 0666);
> -   mfd_assert_has_seals(fd, F_SEAL_EXEC);
> +   mfd_assert_has_seals(fd, F_SEAL_SEAL | F_SEAL_EXEC);
> mfd_fail_chmod(fd, 0777);
> close(fd);
>  }
> --
> 2.45.0
>

Reviewed-by: Jeff Xu 

Thanks!
-Jeff



Re: [PATCH v2 1/2] selftests/resctrl: Adjust effective L3 cache size with SNC enabled

2024-05-16 Thread Maciej Wieczor-Retman
On 2024-05-15 at 16:48:44 +, Luck, Tony wrote:
>If/when my SNC patches go upstream the SNC check could become:
>
>snc_ways=$(ls -d /sys/fs/resctrl/mon_data/mon_L3_00/mon_sub_L3_* 2>/dev/null | 
>wc -l)

But this won't work without your kernel patches right? 

If they are already in the kernel used by the person launching the selftests
then there shouldn't be any problems to report. The idea was that if the
CMT/MBM/MBA selftests fail, the message with "SNC is the problem" is only
displayed if SNC is enabled and there is no kernel support for SNC.

>
>assuming you have /sys/fs/resctrl mounted.
>
>-Tony

-- 
Kind regards
Maciej Wieczór-Retman