Enhanced Timer Handling and Testing for One Shot Timer

[hckuo]

This pull request introduces improvements to timer handling and testing:

1. Timer Handling Updates

   • Modified the itimer_armed macro in restorer.c to properly consider the it_value field.
   • Simplified the decode_itimer function in timer.c to utilize the provided it_value.
   • Removed conditional logic that previously set it_value to interval, ensuring more accurate timer behavior.

2. ZDTM Test Enhancements

   • Refactored setup_timers() function to accept a timer configuration parameter, allowing for more flexible testing scenarios.
   • Introduced a one-shot timer configuration in addition to the existing periodic timer.
   • Expanded the test suite to run twice, each time with a different timer configuration, providing more comprehensive coverage.

[avagin]

The test passes without the fix:

$ git log HEAD~3... --pretty=oneline
36bf4a056f3c153baf2b130cac6df8f62835e3b4 (HEAD) Revert "Update timer handling to properly respect the it_value field"
c20a4cfa3a403558dd702942cf02b51b36c01f30 test: zdtm: Enhance timers test with multiple timer configurations
3249a8308806ba9e5a0dc0501268c5b40833d721 Update timer handling to properly respect the it_value field
$ make -j 4
$ python test/zdtm.py run -t zdtm/static/timers --ignore-taint -f h
userns is supported
The kernel is tainted: '12352'
=== Run 1/1 ================ zdtm/static/timers
========================= Run zdtm/static/timers in h ==========================
Start test
./timers --pidfile=timers.pid --outfile=timers.out
Run criu dump
Run criu restore
Send the 15 signal to  52
Wait for zdtm/static/timers(52) to die for 0.100000
Wait for zdtm/static/timers(52) to die for 0.200000
Wait for zdtm/static/timers(52) to die for 0.400000
Wait for zdtm/static/timers(52) to die for 0.800000
Wait for zdtm/static/timers(52) to die for 1.600000
Removing dump/zdtm/static/timers/52
========================= Test zdtm/static/timers PASS =========================

I think the intention was to introduce the regression test, wasn't it?

[hckuo]

I think it has something to do with how the test is setup. If I only test the ones hot timers it would fail as expected. I can duplicate the test file so one for regular timers and one for one shot timers. However this is something that I want to avoid to not have duplicate code. Please instruct on a better way if any. Or I can just duplicate the test file and make it two tests.

…

On Fri, Jan 10, 2025 at 18:32 Andrei Vagin ***@***.***> wrote: The test passes without the fix: $ git log HEAD~3... --pretty=oneline 36bf4a056f3c153baf2b130cac6df8f62835e3b4 (HEAD) Revert "Update timer handling to properly respect the it_value field" c20a4cf test: zdtm: Enhance timers test with multiple timer configurations 3249a83 Update timer handling to properly respect the it_value field $ make -j 4 $ python test/zdtm.py run -t zdtm/static/timers --ignore-taint -f h userns is supported The kernel is tainted: '12352' === Run 1/1 ================ zdtm/static/timers ========================= Run zdtm/static/timers in h ========================== Start test ./timers --pidfile=timers.pid --outfile=timers.out Run criu dump Run criu restore Send the 15 signal to 52 Wait for zdtm/static/timers(52) to die for 0.100000 Wait for zdtm/static/timers(52) to die for 0.200000 Wait for zdtm/static/timers(52) to die for 0.400000 Wait for zdtm/static/timers(52) to die for 0.800000 Wait for zdtm/static/timers(52) to die for 1.600000 Removing dump/zdtm/static/timers/52 ========================= Test zdtm/static/timers PASS ========================= I think the intention was to introduce the regression test, wasn't it? — Reply to this email directly, view it on GitHub <#2562 (comment)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/AZADMJQ6IGNARJYKK5QBZHD2KB7FPAVCNFSM6AAAAABU25D5QGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBVGAZDANBUGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[hckuo]

ubuntu@ip-172-31-13-102:~/criu$ git log HEAD~3... --pretty=oneline
41298c8888a4eb11578c5fa372647982f9412a9b (HEAD -> fix-one-shot-itimer) Revert "Update timer handling to properly respect the it_value field"
c20a4cfa3a403558dd702942cf02b51b36c01f30 (origin/fix-one-shot-itimer) test: zdtm: Enhance timers test with multiple timer configurations
3249a8308806ba9e5a0dc0501268c5b40833d721 Update timer handling to properly respect the it_value field
ubuntu@ip-172-31-13-102:~/criu$ git diff
diff --git a/test/zdtm/static/timers.c b/test/zdtm/static/timers.c
index 5a919f2b5..95004f384 100644
--- a/test/zdtm/static/timers.c
+++ b/test/zdtm/static/timers.c
@@ -87,7 +87,7 @@ int main(int argc, char **argv)
        };
 
        test_init(argc, argv);
-       for (int i = 0; i < 2; i++) {
+       for (int i = 1; i < 2; i++) {
                setup_timers(&tvs[i]);
 
                test_daemon();
ubuntu@ip-172-31-13-102:~/criu$ sudo python3 test/zdtm.py run -t zdtm/static/timers --ignore-taint -f h
userns is supported
=== Run 1/1 ================ zdtm/static/timers
========================= Run zdtm/static/timers in h ==========================
 DEP       timers.d
 CC        timers.o
 DEP       parseargs.d
 CC        parseargs.o
 AR        libzdtmtst.a
 LINK      groups
 LINK      timers
Start test
./timers --pidfile=timers.pid --outfile=timers.out
Run criu dump
Run criu restore
Send the 15 signal to  116
Wait for zdtm/static/timers(116) to die for 0.100000
Wait for zdtm/static/timers(116) to die for 0.200000
Wait for zdtm/static/timers(116) to die for 0.400000
Wait for zdtm/static/timers(116) to die for 0.800000
Wait for zdtm/static/timers(116) to die for 1.600000
Wait for zdtm/static/timers(116) to die for 3.200000
Wait for zdtm/static/timers(116) to die for 6.400000
==== ALARM ====
################# Test zdtm/static/timers FAIL at result check #################
Test output: ================================
22:21:52.061:   116: FAIL: timers.c:68: timer 0 stuck (errno = 11 (Resource temporarily unavailable))

 <<< ================================
##################################### FAIL #####################################

[avagin]

This code handles the case when the timer is armed, but it_value is set to 0:
https://man7.org/linux/man-pages/man2/setitimer.2.html

 If  both fields of it_value are zero, then this timer is currently disarmed (inactive).

[avagin]

nit: The limit on the length of lines is 80 columns and this is a strongly preferred limit.

[avagin]

actually, you need to check just it_value.tv_sec and it_value.tv_usec:

#define itimer_armed(args, i) (args->itimers[i].it_value.tv_sec || args->itimers[i].it_value.tv_usec)

If this two fields are zero, setitimer disarms the timer.

[avagin]

It will not work. test_waitsig does its job when it is called first time. If you call it second time, it returns immediately:

criu/test/zdtm/lib/test.c

Line 365 in 27a5b9a

futex_wait_while(&sig_received, 0);

I suggest to add a new test. Here is my version of it: avagin@c02e313

It fails without the your fix and it passes with it:

# ./test/zdtm.py run -t zdtm/static/timers01 --ignore-taint
userns is supported
The kernel is tainted: '12352'
=== Run 1/1 ================ zdtm/static/timers01
======================= Run zdtm/static/timers01 in uns ========================
Start test
./timers01 --pidfile=timers01.pid --outfile=timers01.out
Run criu dump
Run criu restore
=[log]=> dump/zdtm/static/timers01/64/1/restore.log
------------------------ grep Error ------------------------
b'(00.009848)      1: No ipcns-sem-11.img image'
b'(00.018559)      1: net: Try to restore a link 10:1:lo'
b'(00.018566)      1: net: Restoring link lo type 1'
b'(00.021284)      1: net: \tRunning ip addr restore'
b'Error: ipv4: Address already assigned.'
b'Error: ipv6: address already assigned.'
------------------------ ERROR OVER ------------------------
Send the 15 signal to  92
Wait for zdtm/static/timers01(92) to die for 0.100000
################ Test zdtm/static/timers01 FAIL at result check ################
Test output: ================================
17:44:11.873:     4: FAIL: timers01.c:54: 0 isn't in [3600, 3300] (errno = 11 (Resource temporarily unavailable))

 <<< ================================
##################################### FAIL #####################################