On Thu, Oct 21, 2021 at 08:39:05AM +0800, Ming Lei wrote:
On Wed, Oct 20, 2021 at 08:48:04AM -0700, Luis Chamberlain wrote:
A second series of tests is hitting CTRL-C on either randonly and restarting testing once again randomly.
ltp/zram02.sh has cleanup handler via trap to clean everything(swapoff/umount/reset/ rmmod), ctrl-c will terminate current forground task and cause shell to run the cleanup handler first, but further 'ctrl-c' will terminate the cleanup handler, then the cleanup won't be done completely, such as zram disk is left as swap device and zram can't be unloaded. The idea can be observed via the following script:
#!/bin/bash trap 'echo "enter trap"; sleep 20; echo "exit trap";' INT sleep 30
After the above script is run foreground, when 1st ctrl-c is pressed, 'sleep 30' is terminated, then the trap command is run, so you can see "enter trap" dumped. Then if you pressed 2nd ctrl-c, 'sleep 20' is terminated immediately. So 'swapoff' from zram02.sh's trap function can be terminated in this way.
zram disk being left as swap disk can be observed with your patch too after terminating via multiple ctrl-c which has to be done this way because the test is dead loop.
So it is hard to cleanup everything completely after multiple 'CTRL-C' is involved, and it should be impossible. It needs violent multiple ctrl-c to terminate the dealoop test.
So it isn't reasonable to expect that zram can be always unloaded successfully after the test script is terminated via multiple ctrl-c.
For the life of me, I do not run into these issue with my patch. But with yours I had.
To be clear, I run zram02.sh on two terminals. Then to interrupt I just leave CTRL-C pressed to issue multiple terminations until the script is done on each terminal at a time, until I see both have completed.
I repeat the same test, noting always that when I start one one terminal the test is succeeding. And also when I cancel completely one script the test continue fine without issue.
But zram can be unloaded after running swapoff manually, from driver viewpoint, nothing is wrong.
I had not run into that issue with my patch FWIW.
Luis