diff mbox series

selftests/livepatch: wait for atomic replace to occur

Message ID 20240822173122.14760-1-rysulliv@redhat.com (mailing list archive)
State Accepted
Commit 052f3951640fd96d2e777b3272a925ec6c0c8100
Headers show
Series selftests/livepatch: wait for atomic replace to occur | expand

Commit Message

Ryan B. Sullivan Aug. 22, 2024, 5:31 p.m. UTC
On some machines with a large number of CPUs there is a sizable delay
between an atomic replace occurring and when sysfs updates accordingly.
This fix uses 'loop_until' to wait for the atomic replace to unload all
previous livepatches.

Signed-off-by: Ryan Sullivan <rysulliv@redhat.com>
---
 tools/testing/selftests/livepatch/test-livepatch.sh | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Petr Mladek Aug. 23, 2024, noon UTC | #1
Hi,

this is 2nd version of the patch. There should have been used
[PATCH v2] in the Subject to make it clear in the mailbox.

On Thu 2024-08-22 13:31:22, Ryan Sullivan wrote:
> On some machines with a large number of CPUs there is a sizable delay
> between an atomic replace occurring and when sysfs updates accordingly.
> This fix uses 'loop_until' to wait for the atomic replace to unload all
> previous livepatches.
> 

I think that Joe suggested to add:

Reported-by: CKI Project <cki-project@redhat.com>
Closes: https://datawarehouse.cki-project.org/kcidb/tests/redhat:1413102084-x86_64-kernel_upt_28

> Signed-off-by: Ryan Sullivan <rysulliv@redhat.com>
> ---

Also it is a good practice to summarize changes between versions.
In this case it would have been something like:

Changes against v1:

  - Cleaned the commit message.

>  tools/testing/selftests/livepatch/test-livepatch.sh | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
> index 65c9c058458d..bd13257bfdfe 100755
> --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> @@ -139,11 +139,8 @@ load_lp $MOD_REPLACE replace=1
>  grep 'live patched' /proc/cmdline > /dev/kmsg
>  grep 'live patched' /proc/meminfo > /dev/kmsg
>  
> -mods=(/sys/kernel/livepatch/*)
> -nmods=${#mods[@]}
> -if [ "$nmods" -ne 1 ]; then
> -	die "Expecting only one moduled listed, found $nmods"
> -fi
> +loop_until 'mods=(/sys/kernel/livepatch/*); nmods=${#mods[@]}; [[ "$nmods" -eq 1 ]]' ||
> +        die "Expecting only one moduled listed, found $nmods"
>  
>  # These modules were disabled by the atomic replace
>  for mod in $MOD_LIVEPATCH3 $MOD_LIVEPATCH2 $MOD_LIVEPATCH1; do

Otherwise, it looks good to me. With the added references:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

PS: No need to resend the patch. I would add the references when
    committing. I am going to wait few more days before committing.
Ryan B. Sullivan Aug. 23, 2024, 1:09 p.m. UTC | #2
Changes from v2:

Adds:
	Reported-by: CKI Project <cki-project@redhat.com>
	Closes: https://datawarehouse.cki-project.org/kcidb/tests/redhat:1413102084-x86_64-kernel_upt_28
From 9d9bfb21e86a3a79fb92fd22d927329510c6a672 Mon Sep 17 00:00:00 2001
From: Ryan Sullivan <rysulliv@redhat.com>
Date: Thu, 22 Aug 2024 12:19:54 -0400
Subject: [PATCH v3] selftests/livepatch: wait for atomic replace to occur

On some machines with a large number of CPUs there is a sizable delay
between an atomic replace occurring and when sysfs updates accordingly.
This fix uses 'loop_until' to wait for the atomic replace to unload all
previous livepatches.

Reported-by: CKI Project <cki-project@redhat.com>
Closes: https://datawarehouse.cki-project.org/kcidb/tests/redhat:1413102084-x86_64-kernel_upt_28
Signed-off-by: Ryan Sullivan <rysulliv@redhat.com>
---
 tools/testing/selftests/livepatch/test-livepatch.sh | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 65c9c058458d..bd13257bfdfe 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -139,11 +139,8 @@ load_lp $MOD_REPLACE replace=1
 grep 'live patched' /proc/cmdline > /dev/kmsg
 grep 'live patched' /proc/meminfo > /dev/kmsg
 
-mods=(/sys/kernel/livepatch/*)
-nmods=${#mods[@]}
-if [ "$nmods" -ne 1 ]; then
-	die "Expecting only one moduled listed, found $nmods"
-fi
+loop_until 'mods=(/sys/kernel/livepatch/*); nmods=${#mods[@]}; [[ "$nmods" -eq 1 ]]' ||
+        die "Expecting only one moduled listed, found $nmods"
 
 # These modules were disabled by the atomic replace
 for mod in $MOD_LIVEPATCH3 $MOD_LIVEPATCH2 $MOD_LIVEPATCH1; do
Petr Mladek Aug. 26, 2024, 1:41 p.m. UTC | #3
Hi,

JFYI, I have committed the patch into livepatching.git,
branch for-6.11/selftests-fixup.

Few nits below :-)

On Fri 2024-08-23 09:09:26, Ryan B. Sullivan wrote:
> Changes from v2:
> 
> Adds:
> 	Reported-by: CKI Project <cki-project@redhat.com>
> 	Closes: https://datawarehouse.cki-project.org/kcidb/tests/redhat:1413102084-x86_64-kernel_upt_28

The "changelog" should have been added after the "---" line.

It allows to send the patch as the main part of the mail (no attachment).
The lines below the "---" gets automatically removed when the "mail"
gets applied using "git am".

Best Regards,
Petr
diff mbox series

Patch

diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 65c9c058458d..bd13257bfdfe 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -139,11 +139,8 @@  load_lp $MOD_REPLACE replace=1
 grep 'live patched' /proc/cmdline > /dev/kmsg
 grep 'live patched' /proc/meminfo > /dev/kmsg
 
-mods=(/sys/kernel/livepatch/*)
-nmods=${#mods[@]}
-if [ "$nmods" -ne 1 ]; then
-	die "Expecting only one moduled listed, found $nmods"
-fi
+loop_until 'mods=(/sys/kernel/livepatch/*); nmods=${#mods[@]}; [[ "$nmods" -eq 1 ]]' ||
+        die "Expecting only one moduled listed, found $nmods"
 
 # These modules were disabled by the atomic replace
 for mod in $MOD_LIVEPATCH3 $MOD_LIVEPATCH2 $MOD_LIVEPATCH1; do