RE: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
>+prlog "Causing kernel crash ..."
>+
>+# enable all functions triggered by sysrq
>+echo 1 > /proc/sys/kernel/sysrq
>+# setting to reboot in 3 seconds after panic
>+echo 3 > /proc/sys/kernel/panic
>+# setting to cause panic when oops occurs
>+echo 1 > /proc/sys/kernel/panic_on_oops
>+
>+# create a file as reboot flag
>+touch $REBOOT_FILE
>+sync
>+
>+# cause crash
>+echo c > /proc/sysrq-trigger
Do you need to stop kdump service before the sysrq?
Or, does it cover oops and kdump case?
Seiji
> -Original Message-
> From: 豊岡拓 / Toyooka，Hiraku
> Sent: Tuesday, September 15, 2015 11:42 AM
> To: Kees Cook
> Cc: LKML; Tony Luck; Linux API; Anton Vorontsov; Shuah Khan; Mark Salyzyn;
> Colin Cross; 阿口誠司 / AGUCHI，SEIJI
> Subject: Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with
> reboot
>
> Hello Kees,
>
> >> +run_crash:
> >> + @sh pstore_crash_test || echo "pstore_crash_test: [FAIL]"
> >
> > This is probably better written to exit 1 on failure, otherwise it
> > just _says_ it fails. (Though lots of selftests in the tree already
> > have this problem, it's best to avoid the pattern for new stuff.)
> > Maybe something like:
> >
> > @sh pstore_crash_test || { echo "pstore_crash_test: [FAIL]";
> exit 1; }
>
> OK. I'll add the "exit 1".
>
> >> +prlog -n "Checking dmesg files exist in pstore filesystem ... "
> >> +if [ -e dmesg-${backend}-0 ]; then
> >> +prlog "ok"
> >> +for f in `ls dmesg-${backend}-*`; do
> >> + prlog -e "\t${f}"
> >> +done
> >> +else
> >> +prlog "FAIL"
> >> +rc=1
> >> +fi
> >
> > This test pattern is repeated a lot. Maybe better to create a helper
> > function instead? It could make the tests much more readable.
>
> Yes, I should make a helper function in v2.
>
> Best regards,
> Hiraku Toyooka
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
Hi,
> >> +prlog -n "Checking pstore backend is registered ... "
> >> +be_msg=`dmesg | grep "pstore: Registered [a-zA-Z0-9]\+ as persistent
> >> store backend$"`
> >> +if [ $? -eq 0 ]; then
> >> +backend=`echo ${be_msg} | sed -e 's/^.*Registered\
> >> \([a-zA-z0-9-]\+\)\ as.*$/\1/g'`
> >> +prlog "ok"
> >> +else
> >> +prlog "FAIL"
> >> +exit 1
It may be good if you can log "/sys/module/pstore/parameters/backend/"
or /proc/cmdline in failure case.
It makes debug easy.
Seiji
> >> +fi
> -Original Message-
> From: 豊岡拓 / Toyooka，Hiraku
> Sent: Tuesday, September 15, 2015 11:31 AM
> To: Kees Cook
> Cc: LKML; Tony Luck; Linux API; Anton Vorontsov; Shuah Khan; Mark Salyzyn;
> Colin Cross; 阿口誠司 / AGUCHI，SEIJI
> Subject: Re: [PATCH 1/2] selftests/pstore: add pstore test script for
> pre-reboot
>
> Hello, Kees,
>
> Thank you for your advise.
>
> >> +be_msg=`dmesg | grep "pstore: Registered [a-zA-Z0-9]\+ as
> persistent store backend$"`
> ...
> > This seems unstable if the system hasn't booted recently or if stuff
> > is spamming dmesg. What about examining /sys/module/pstore instead?
>
> OK, I'll update in that way.
>
> Best regards,
> Hiraku Toyooka
>
> Kees Cook wrote:
> > On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
> > wrote:
> >> The pstore_tests script includes test cases which check pstore's
> >> behavior before crash (and reboot).
> >>
> >> The test cases are currently following.
> >>
> >> - Check pstore backend is registered
> >> - Check pstore console is registered
> >> - Check /dev/pmsg0 exists
> >> - Write string to /dev/pmsg0
> >>
> >> Example usage is following.
> >>
> >> make: Entering directory '/home/root/selftests/pstore'
> >> === Pstore unit tests (pstore_tests)===
> >> Checking pstore backend is registered ... ok
> >> Checking pstore console is registered ... ok
> >> Checking /dev/pmsg0 exists ... ok
> >> Writing TEST_STRING to /dev/pmsg0 ... ok
> >> selftests: pstore_tests [PASS]
> >> === Pstore unit tests (pstore_post_reboot_tests)===
> >> Checking pstore backend is registered ... ok
> >> pstore_crash_test has not been executed yet. we skip further tests.
> >> selftests: pstore_post_reboot_tests [PASS]
> >> make: Leaving directory '/home/root/selftests/pstore'
> >>
> >> We can also see test logs later.
> >>
> >> Signed-off-by: Hiraku Toyooka
> >> Cc: Shuah Khan
> >> Cc: Tony Luck
> >> Cc: Anton Vorontsov
> >> Cc: Colin Cross
> >> Cc: Kees Cook
> >> Cc: Mark Salyzyn
> >> Cc: Seiji Aguchi
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-...@vger.kernel.org
> >> ---
> >> tools/testing/selftests/Makefile|1 +
> >> tools/testing/selftests/pstore/Makefile | 12 +++
> >> tools/testing/selftests/pstore/common_tests | 45
> >> +++
> >> tools/testing/selftests/pstore/pstore_tests | 42
> >> +
> >> 4 files changed, 100 insertions(+)
> >> create mode 100644 tools/testing/selftests/pstore/Makefile
> >> create mode 100755 tools/testing/selftests/pstore/common_tests
> >> create mode 100755 tools/testing/selftests/pstore/pstore_tests
> >>
> >> diff --git a/tools/testing/selftests/Makefile
> >> b/tools/testing/selftests/Makefile
> >> index 24ae9e8..b58c72e 100644
> >> --- a/tools/testing/selftests/Makefile
> >> +++ b/tools/testing/selftests/Makefile
> >> @@ -12,6 +12,7 @@ TARGETS += mount
> >> TARGETS += mqueue
> >> TARGETS += net
> >> TARGETS += powerpc
> >> +TARGETS += pstore
> >> TARGETS += ptrace
> >> TARGETS += seccomp
> >> TARGETS += size
> >> diff --git a/tools/testing/selftests/pstore/Makefile
> >> b/tools/testing/selftests/pstore/Makefile
> >> new file mode 100644
> >> index 000..40b887d
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/Makefile
> >> @@ -0,0 +1,12 @@
> >> +# Makefile for pstore selftests.
> >> +# Expects pstore backend is registered.
> >> +
> >> +all:
> >> +
> >> +TEST_PROGS := pstore_tests
> >> +TEST_FILES := common_tests
> >> +
> >> +include ../lib.mk
> >>
RE: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
Hi,
> >> +prlog -n "Checking pstore backend is registered ... "
> >> +be_msg=`dmesg | grep "pstore: Registered [a-zA-Z0-9]\+ as persistent
> >> store backend$"`
> >> +if [ $? -eq 0 ]; then
> >> +backend=`echo ${be_msg} | sed -e 's/^.*Registered\
> >> \([a-zA-z0-9-]\+\)\ as.*$/\1/g'`
> >> +prlog "ok"
> >> +else
> >> +prlog "FAIL"
> >> +exit 1
It may be good if you can log "/sys/module/pstore/parameters/backend/"
or /proc/cmdline in failure case.
It makes debug easy.
Seiji
> >> +fi
> -Original Message-
> From: 豊岡拓 / Toyooka，Hiraku
> Sent: Tuesday, September 15, 2015 11:31 AM
> To: Kees Cook
> Cc: LKML; Tony Luck; Linux API; Anton Vorontsov; Shuah Khan; Mark Salyzyn;
> Colin Cross; 阿口誠司 / AGUCHI，SEIJI
> Subject: Re: [PATCH 1/2] selftests/pstore: add pstore test script for
> pre-reboot
>
> Hello, Kees,
>
> Thank you for your advise.
>
> >> +be_msg=`dmesg | grep "pstore: Registered [a-zA-Z0-9]\+ as
> persistent store backend$"`
> ...
> > This seems unstable if the system hasn't booted recently or if stuff
> > is spamming dmesg. What about examining /sys/module/pstore instead?
>
> OK, I'll update in that way.
>
> Best regards,
> Hiraku Toyooka
>
> Kees Cook wrote:
> > On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
> > <hiraku.toyooka...@hitachi.com> wrote:
> >> The pstore_tests script includes test cases which check pstore's
> >> behavior before crash (and reboot).
> >>
> >> The test cases are currently following.
> >>
> >> - Check pstore backend is registered
> >> - Check pstore console is registered
> >> - Check /dev/pmsg0 exists
> >> - Write string to /dev/pmsg0
> >>
> >> Example usage is following.
> >>
> >> make: Entering directory '/home/root/selftests/pstore'
> >> === Pstore unit tests (pstore_tests)===
> >> Checking pstore backend is registered ... ok
> >> Checking pstore console is registered ... ok
> >> Checking /dev/pmsg0 exists ... ok
> >> Writing TEST_STRING to /dev/pmsg0 ... ok
> >> selftests: pstore_tests [PASS]
> >> === Pstore unit tests (pstore_post_reboot_tests)===
> >> Checking pstore backend is registered ... ok
> >> pstore_crash_test has not been executed yet. we skip further tests.
> >> selftests: pstore_post_reboot_tests [PASS]
> >> make: Leaving directory '/home/root/selftests/pstore'
> >>
> >> We can also see test logs later.
> >>
> >> Signed-off-by: Hiraku Toyooka <hiraku.toyooka...@hitachi.com>
> >> Cc: Shuah Khan <shua...@osg.samsung.com>
> >> Cc: Tony Luck <tony.l...@intel.com>
> >> Cc: Anton Vorontsov <an...@enomsg.org>
> >> Cc: Colin Cross <ccr...@android.com>
> >> Cc: Kees Cook <keesc...@chromium.org>
> >> Cc: Mark Salyzyn <saly...@android.com>
> >> Cc: Seiji Aguchi <seiji.agu...@hds.com>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-...@vger.kernel.org
> >> ---
> >> tools/testing/selftests/Makefile|1 +
> >> tools/testing/selftests/pstore/Makefile | 12 +++
> >> tools/testing/selftests/pstore/common_tests | 45
> >> +++
> >> tools/testing/selftests/pstore/pstore_tests | 42
> >> +
> >> 4 files changed, 100 insertions(+)
> >> create mode 100644 tools/testing/selftests/pstore/Makefile
> >> create mode 100755 tools/testing/selftests/pstore/common_tests
> >> create mode 100755 tools/testing/selftests/pstore/pstore_tests
> >>
> >> diff --git a/tools/testing/selftests/Makefile
> >> b/tools/testing/selftests/Makefile
> >> index 24ae9e8..b58c72e 100644
> >> --- a/tools/testing/selftests/Makefile
> >> +++ b/tools/testing/selftests/Makefile
> >> @@ -12,6 +12,7 @@ TARGETS += mount
> >> TARGETS += mqueue
> >> TARGETS += net
> >> TARGETS += powerpc
> >> +TARGETS += pstore
> >> TARGETS += ptrace
> >> TARGETS += seccomp
> >> TARGETS += size
> >> diff --git a/tools/testing/selftests/pstore/Makefile
> >> b/tools/testing/selftests/pstore/Makefile
> >> new file mode 100644
> >> index 000..40b887d
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/Makefile
> >> @@ -0,0 +1,12 @@
> >> +# Makefil
RE: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
>+prlog "Causing kernel crash ..."
>+
>+# enable all functions triggered by sysrq
>+echo 1 > /proc/sys/kernel/sysrq
>+# setting to reboot in 3 seconds after panic
>+echo 3 > /proc/sys/kernel/panic
>+# setting to cause panic when oops occurs
>+echo 1 > /proc/sys/kernel/panic_on_oops
>+
>+# create a file as reboot flag
>+touch $REBOOT_FILE
>+sync
>+
>+# cause crash
>+echo c > /proc/sysrq-trigger
Do you need to stop kdump service before the sysrq?
Or, does it cover oops and kdump case?
Seiji
> -Original Message-
> From: 豊岡拓 / Toyooka，Hiraku
> Sent: Tuesday, September 15, 2015 11:42 AM
> To: Kees Cook
> Cc: LKML; Tony Luck; Linux API; Anton Vorontsov; Shuah Khan; Mark Salyzyn;
> Colin Cross; 阿口誠司 / AGUCHI，SEIJI
> Subject: Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with
> reboot
>
> Hello Kees,
>
> >> +run_crash:
> >> + @sh pstore_crash_test || echo "pstore_crash_test: [FAIL]"
> >
> > This is probably better written to exit 1 on failure, otherwise it
> > just _says_ it fails. (Though lots of selftests in the tree already
> > have this problem, it's best to avoid the pattern for new stuff.)
> > Maybe something like:
> >
> > @sh pstore_crash_test || { echo "pstore_crash_test: [FAIL]";
> exit 1; }
>
> OK. I'll add the "exit 1".
>
> >> +prlog -n "Checking dmesg files exist in pstore filesystem ... "
> >> +if [ -e dmesg-${backend}-0 ]; then
> >> +prlog "ok"
> >> +for f in `ls dmesg-${backend}-*`; do
> >> + prlog -e "\t${f}"
> >> +done
> >> +else
> >> +prlog "FAIL"
> >> +rc=1
> >> +fi
> >
> > This test pattern is repeated a lot. Maybe better to create a helper
> > function instead? It could make the tests much more readable.
>
> Yes, I should make a helper function in v2.
>
> Best regards,
> Hiraku Toyooka
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
