All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests/pstore: add pstore test script
@ 2015-09-08 11:06 Hiraku Toyooka
  2015-09-08 11:06 ` [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot Hiraku Toyooka
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-08 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

These scripts include test cases which check pstore behavior. This
is useful to avoid regressions of pstore.

Pstore is used across kernel crash, so these test cases are split
into three parts.

 - pstore_tests: check pstore behavior before crash
 - pstore_post_reboot_tests: check pstore behavior after crash and reboot
 - pstore_crash_test: cause kernel crash and reboot

The pstore_test and the pstore_post_reboot_tests are the actual scripts
for testing pstore and are executed in usual selftest's "run_test" target.
On the other hand, the pstore_crash_test is to cause kernel panic and reboot,
so it is executed in new "run_pstore_crash" target which is specified ad-hoc
by users. In addition, there is a "common_tests" script which includes
utilities and test cases used commonly in these scripts.

When the pstore_crash_test is executed, it creates a file as a reboot flag.
The pstore_post_reboot_tests detects whether the file exists or not. If the
file doesn't exists, the test cases are skipped.

These scripts expect that one pstore backend is registered before the
scripts are executed.
Assumed use case is following.

# cd linux/tools/testing/selftests
# make run_tests -C pstore
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'
# make run_pstore_crash
...
(kernel crash and reboot)
...
# make run_tests -C pstore
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
Mounting pstore filesystem ... ok
Checking dmesg files exist in pstore filesystem ... ok
        dmesg-ramoops-0
        dmesg-ramoops-1
Checking console files exist in pstore filesystem ... ok
        console-ramoops-0
Checking pmsg files exist in pstore filesystem ... ok
        pmsg-ramoops-0
Checking dmesg files contains oops end marker
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
Checking console file contains oops end marker ... ok
Checking pmsg file contains TEST_STRING ... ok
Removing all files in pstore filesystem 
        console-ramoops-0 ... ok
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
        pmsg-ramoops-0 ... ok
selftests: pstore_post_reboot_tests [PASS]
make: Leaving directory '/home/root/selftests/pstore'


We can also see test logs later.

# cat pstore/logs/20150903-111158/pstore_tests.log
...

---

Hiraku Toyooka (2):
      selftests/pstore: add pstore test script for pre-reboot
      selftests/pstore: add pstore test scripts going with reboot


 tools/testing/selftests/Makefile                   |    1 
 tools/testing/selftests/pstore/Makefile            |   15 ++
 tools/testing/selftests/pstore/common_tests        |   46 +++++++
 tools/testing/selftests/pstore/pstore_crash_test   |   27 ++++
 .../selftests/pstore/pstore_post_reboot_tests      |  126 ++++++++++++++++++++
 tools/testing/selftests/pstore/pstore_tests        |   42 +++++++
 6 files changed, 257 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_crash_test
 create mode 100755 tools/testing/selftests/pstore/pstore_post_reboot_tests
 create mode 100755 tools/testing/selftests/pstore/pstore_tests

--
Hiraku Toyooka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
  2015-09-08 11:06 [PATCH 0/2] selftests/pstore: add pstore test script Hiraku Toyooka
@ 2015-09-08 11:06 ` Hiraku Toyooka
  2015-09-08 23:22     ` Mark Salyzyn
  2015-09-08 23:37     ` Kees Cook
  2015-09-08 11:06   ` Hiraku Toyooka
  2015-09-08 23:42   ` Kees Cook
  2 siblings, 2 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-08 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

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.gu@hitachi.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Salyzyn <salyzyn@android.com>
Cc: Seiji Aguchi <seiji.aguchi@hds.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-api@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 0000000..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
+
+clean:
+	rm -rf logs/*
diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
new file mode 100755
index 0000000..98611c5
--- /dev/null
+++ b/tools/testing/selftests/pstore/common_tests
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+# common_tests - Shell script commonly used by pstore test scripts
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
+#
+# Released under the terms of the GPL v2.
+
+# Utilities
+errexit() { # message
+  echo "Error: $1" 1>&2
+  exit 1
+}
+
+absdir() { # file_path
+  (cd `dirname $1`; pwd)
+}
+
+# Parameters
+TOP_DIR=`absdir $0`
+LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
+TEST_STRING="Testing pstore"
+
+# Preparing logs
+LOG_FILE=$LOG_DIR/`basename $0`.log
+mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
+date > $LOG_FILE
+prlog() { # messages
+  /bin/echo "$@" | tee -a $LOG_FILE
+}
+prlog "=== Pstore unit tests (`basename $0`)==="
+
+# Starting tests
+rc=0
+
+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
+fi
diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
new file mode 100755
index 0000000..cbf613c
--- /dev/null
+++ b/tools/testing/selftests/pstore/pstore_tests
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# pstore_tests - Check pstore's behavior before crash/reboot
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
+#
+# Released under the terms of the GPL v2.
+
+. ./common_tests
+
+prlog -n "Checking pstore console is registered ... "
+dmesg | grep -q "console \[pstore"
+if [ $? -eq 0 ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+fi
+
+prlog -n "Checking /dev/pmsg0 exists ... "
+if [ -e "/dev/pmsg0" ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
+if [ -e "/dev/pmsg0" ]; then
+    echo "${TEST_STRING}" > /dev/pmsg0
+    if [ $? -eq 0 ]; then
+	prlog "ok"
+    else
+	prlog "FAIL"
+	rc=1
+    fi
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+exit $rc


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
  2015-09-08 11:06 [PATCH 0/2] selftests/pstore: add pstore test script Hiraku Toyooka
@ 2015-09-08 11:06   ` Hiraku Toyooka
  2015-09-08 11:06   ` Hiraku Toyooka
  2015-09-08 23:42   ` Kees Cook
  2 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-08 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

To test pstore in earnest, we have to cause kernel crash and check
pstore filesystem mouted after reboot.

We add two scripts:
 - pstore_crash_test
     This script to cause crash and reboot easily. It is executed by
     'make run_pstore_crash' in selftests.
 - pstore_post_reboot_tests
     This script includes test cases which check pstore's behavior after
     crash and reboot. It is executed together with pstore_tests by
     'make run_tests [-C pstore]' in selftests.

The test cases in pstore_post_reboot_tests are currently following.

- Check pstore backend is registered
- Mount pstore filesystem
- Check dmesg files exist in pstore filesystem
- Check console file exist in pstore filesystem
- Check pmsg file exist in pstore filesystem
- Check dmesg files contain oops end marker
- Check console file contain oops end marker
- Check pmsg file contain the string written before crash
- Remove all files in pstore filesystem

Example usage is following.

...
(kernel crash and reboot)
...
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
Mounting pstore filesystem ... ok
Checking dmesg files exist in pstore filesystem ... ok
        dmesg-ramoops-0
        dmesg-ramoops-1
Checking console files exist in pstore filesystem ... ok
        console-ramoops-0
Checking pmsg files exist in pstore filesystem ... ok
        pmsg-ramoops-0
Checking dmesg files contains oops end marker
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
Checking console file contains oops end marker ... ok
Checking pmsg file contains TEST_STRING ... ok
Removing all files in pstore filesystem
        console-ramoops-0 ... ok
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
        pmsg-ramoops-0 ... ok
selftests: pstore_post_reboot_tests [PASS]
make: Leaving directory '/home/root/selftests/pstore'

Signed-off-by: Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Salyzyn <salyzyn@android.com>
Cc: Seiji Aguchi <seiji.aguchi@hds.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-api@vger.kernel.org
---
 tools/testing/selftests/pstore/Makefile            |    7 +
 tools/testing/selftests/pstore/common_tests        |    1 
 tools/testing/selftests/pstore/pstore_crash_test   |   27 ++++
 .../selftests/pstore/pstore_post_reboot_tests      |  126 ++++++++++++++++++++
 4 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100755 tools/testing/selftests/pstore/pstore_crash_test
 create mode 100755 tools/testing/selftests/pstore/pstore_post_reboot_tests

diff --git a/tools/testing/selftests/pstore/Makefile b/tools/testing/selftests/pstore/Makefile
index 40b887d..32c408c 100644
--- a/tools/testing/selftests/pstore/Makefile
+++ b/tools/testing/selftests/pstore/Makefile
@@ -3,10 +3,13 @@
 
 all:
 
-TEST_PROGS := pstore_tests
-TEST_FILES := common_tests
+TEST_PROGS := pstore_tests pstore_post_reboot_tests
+TEST_FILES := common_tests pstore_crash_test
 
 include ../lib.mk
 
+run_crash:
+	@sh pstore_crash_test || echo "pstore_crash_test: [FAIL]"
+
 clean:
 	rm -rf logs/*
diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
index 98611c5..8003760 100755
--- a/tools/testing/selftests/pstore/common_tests
+++ b/tools/testing/selftests/pstore/common_tests
@@ -20,6 +20,7 @@ absdir() { # file_path
 # Parameters
 TOP_DIR=`absdir $0`
 LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
+REBOOT_FILE=$TOP_DIR/reboot_flag
 TEST_STRING="Testing pstore"
 
 # Preparing logs
diff --git a/tools/testing/selftests/pstore/pstore_crash_test b/tools/testing/selftests/pstore/pstore_crash_test
new file mode 100755
index 0000000..6d0c422
--- /dev/null
+++ b/tools/testing/selftests/pstore/pstore_crash_test
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+# pstore_crash_test - Pstore test shell script which causes crash and reboot
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
+#
+# Released under the terms of the GPL v2.
+
+# exit if pstore backend is not registered
+. ./common_tests
+
+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
diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
new file mode 100755
index 0000000..0e33366
--- /dev/null
+++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
@@ -0,0 +1,126 @@
+#!/bin/sh
+
+# pstore_post_reboot_tests - Check pstore's behavior after crash/reboot
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
+#
+# Released under the terms of the GPL v2.
+
+. ./common_tests
+
+if [ -e $REBOOT_FILE  ]; then
+    rm $REBOOT_FILE
+else
+    prlog "pstore_crash_test has not been executed yet. we skip further tests."
+    exit 0
+fi
+
+prlog -n "Mounting pstore filesystem ... "
+mount_info=`grep pstore /proc/mounts`
+if [ $? -eq 0 ]; then
+    mount_point=`echo ${mount_info} | cut -d' ' -f2 | head -n1`
+    prlog "ok"
+else
+    mount none /sys/fs/pstore -t pstore
+    if [ $? -eq 0 ]; then
+	mount_point=`grep pstore /proc/mounts | cut -d' ' -f2 | head -n1`
+	prlog "ok"
+    else
+	prlog "FAIL"
+	exit 1
+    fi
+fi
+
+cd ${mount_point}
+
+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
+
+prlog -n "Checking console files exist in pstore filesystem ... "
+if [ -e console-${backend}-0 ]; then
+    prlog "ok"
+    for f in `ls console-${backend}-*`; do
+	prlog -e "\t${f}"
+    done
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking pmsg files exist in pstore filesystem ... "
+if [ -e pmsg-${backend}-0 ]; then
+    prlog "ok"
+    for f in `ls pmsg-${backend}-*`; do
+	prlog -e "\t${f}"
+    done
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking dmesg files contains oops end marker"
+files=`ls dmesg-${backend}-*`
+if [ $? -eq 0 ]; then
+    prlog
+    for f in $files; do
+	prlog -ne "\t${f} ... "
+	grep -q "\---\[ end trace" $f
+	if [ $? -eq 0 ]; then
+	    prlog "ok"
+	else
+	    prlog "FAIL"
+	    rc=1
+	fi
+    done
+else
+    prlog " ... FAIL"
+    rc=1
+fi
+
+prlog -n "Checking console file contains oops end marker ... "
+grep -q "\---\[ end trace" console-${backend}-0
+if [ $? -eq 0 ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking pmsg file contains TEST_STRING ... "
+grep -q "${TEST_STRING}" pmsg-${backend}-0
+if [ $? -eq 0 ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Removing all files in pstore filesystem "
+files=`ls *-${backend}-*`
+if [ $? -eq 0 ]; then
+    prlog
+    for f in ${files}; do
+	prlog -ne "\t${f} ... "
+	rm ${f}
+	if [ $? -eq 0 ]; then
+	    prlog "ok"
+	else
+	    prlog "FAIL"
+	    rc=1
+	fi
+    done
+else
+    prlog " ... FAIL"
+    rc=1
+fi
+
+exit $rc


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-08 11:06   ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-08 11:06 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Tony Luck, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Anton Vorontsov, Shuah Khan, Mark Salyzyn, Colin Cross,
	Seiji Aguchi

To test pstore in earnest, we have to cause kernel crash and check
pstore filesystem mouted after reboot.

We add two scripts:
 - pstore_crash_test
     This script to cause crash and reboot easily. It is executed by
     'make run_pstore_crash' in selftests.
 - pstore_post_reboot_tests
     This script includes test cases which check pstore's behavior after
     crash and reboot. It is executed together with pstore_tests by
     'make run_tests [-C pstore]' in selftests.

The test cases in pstore_post_reboot_tests are currently following.

- Check pstore backend is registered
- Mount pstore filesystem
- Check dmesg files exist in pstore filesystem
- Check console file exist in pstore filesystem
- Check pmsg file exist in pstore filesystem
- Check dmesg files contain oops end marker
- Check console file contain oops end marker
- Check pmsg file contain the string written before crash
- Remove all files in pstore filesystem

Example usage is following.

...
(kernel crash and reboot)
...
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
Mounting pstore filesystem ... ok
Checking dmesg files exist in pstore filesystem ... ok
        dmesg-ramoops-0
        dmesg-ramoops-1
Checking console files exist in pstore filesystem ... ok
        console-ramoops-0
Checking pmsg files exist in pstore filesystem ... ok
        pmsg-ramoops-0
Checking dmesg files contains oops end marker
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
Checking console file contains oops end marker ... ok
Checking pmsg file contains TEST_STRING ... ok
Removing all files in pstore filesystem
        console-ramoops-0 ... ok
        dmesg-ramoops-0 ... ok
        dmesg-ramoops-1 ... ok
        pmsg-ramoops-0 ... ok
selftests: pstore_post_reboot_tests [PASS]
make: Leaving directory '/home/root/selftests/pstore'

Signed-off-by: Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Mark Salyzyn <salyzyn-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 tools/testing/selftests/pstore/Makefile            |    7 +
 tools/testing/selftests/pstore/common_tests        |    1 
 tools/testing/selftests/pstore/pstore_crash_test   |   27 ++++
 .../selftests/pstore/pstore_post_reboot_tests      |  126 ++++++++++++++++++++
 4 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100755 tools/testing/selftests/pstore/pstore_crash_test
 create mode 100755 tools/testing/selftests/pstore/pstore_post_reboot_tests

diff --git a/tools/testing/selftests/pstore/Makefile b/tools/testing/selftests/pstore/Makefile
index 40b887d..32c408c 100644
--- a/tools/testing/selftests/pstore/Makefile
+++ b/tools/testing/selftests/pstore/Makefile
@@ -3,10 +3,13 @@
 
 all:
 
-TEST_PROGS := pstore_tests
-TEST_FILES := common_tests
+TEST_PROGS := pstore_tests pstore_post_reboot_tests
+TEST_FILES := common_tests pstore_crash_test
 
 include ../lib.mk
 
+run_crash:
+	@sh pstore_crash_test || echo "pstore_crash_test: [FAIL]"
+
 clean:
 	rm -rf logs/*
diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
index 98611c5..8003760 100755
--- a/tools/testing/selftests/pstore/common_tests
+++ b/tools/testing/selftests/pstore/common_tests
@@ -20,6 +20,7 @@ absdir() { # file_path
 # Parameters
 TOP_DIR=`absdir $0`
 LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
+REBOOT_FILE=$TOP_DIR/reboot_flag
 TEST_STRING="Testing pstore"
 
 # Preparing logs
diff --git a/tools/testing/selftests/pstore/pstore_crash_test b/tools/testing/selftests/pstore/pstore_crash_test
new file mode 100755
index 0000000..6d0c422
--- /dev/null
+++ b/tools/testing/selftests/pstore/pstore_crash_test
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+# pstore_crash_test - Pstore test shell script which causes crash and reboot
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
+#
+# Released under the terms of the GPL v2.
+
+# exit if pstore backend is not registered
+. ./common_tests
+
+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
diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
new file mode 100755
index 0000000..0e33366
--- /dev/null
+++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
@@ -0,0 +1,126 @@
+#!/bin/sh
+
+# pstore_post_reboot_tests - Check pstore's behavior after crash/reboot
+#
+# Copyright (C) Hitachi Ltd., 2015
+#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
+#
+# Released under the terms of the GPL v2.
+
+. ./common_tests
+
+if [ -e $REBOOT_FILE  ]; then
+    rm $REBOOT_FILE
+else
+    prlog "pstore_crash_test has not been executed yet. we skip further tests."
+    exit 0
+fi
+
+prlog -n "Mounting pstore filesystem ... "
+mount_info=`grep pstore /proc/mounts`
+if [ $? -eq 0 ]; then
+    mount_point=`echo ${mount_info} | cut -d' ' -f2 | head -n1`
+    prlog "ok"
+else
+    mount none /sys/fs/pstore -t pstore
+    if [ $? -eq 0 ]; then
+	mount_point=`grep pstore /proc/mounts | cut -d' ' -f2 | head -n1`
+	prlog "ok"
+    else
+	prlog "FAIL"
+	exit 1
+    fi
+fi
+
+cd ${mount_point}
+
+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
+
+prlog -n "Checking console files exist in pstore filesystem ... "
+if [ -e console-${backend}-0 ]; then
+    prlog "ok"
+    for f in `ls console-${backend}-*`; do
+	prlog -e "\t${f}"
+    done
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking pmsg files exist in pstore filesystem ... "
+if [ -e pmsg-${backend}-0 ]; then
+    prlog "ok"
+    for f in `ls pmsg-${backend}-*`; do
+	prlog -e "\t${f}"
+    done
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking dmesg files contains oops end marker"
+files=`ls dmesg-${backend}-*`
+if [ $? -eq 0 ]; then
+    prlog
+    for f in $files; do
+	prlog -ne "\t${f} ... "
+	grep -q "\---\[ end trace" $f
+	if [ $? -eq 0 ]; then
+	    prlog "ok"
+	else
+	    prlog "FAIL"
+	    rc=1
+	fi
+    done
+else
+    prlog " ... FAIL"
+    rc=1
+fi
+
+prlog -n "Checking console file contains oops end marker ... "
+grep -q "\---\[ end trace" console-${backend}-0
+if [ $? -eq 0 ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Checking pmsg file contains TEST_STRING ... "
+grep -q "${TEST_STRING}" pmsg-${backend}-0
+if [ $? -eq 0 ]; then
+    prlog "ok"
+else
+    prlog "FAIL"
+    rc=1
+fi
+
+prlog -n "Removing all files in pstore filesystem "
+files=`ls *-${backend}-*`
+if [ $? -eq 0 ]; then
+    prlog
+    for f in ${files}; do
+	prlog -ne "\t${f} ... "
+	rm ${f}
+	if [ $? -eq 0 ]; then
+	    prlog "ok"
+	else
+	    prlog "FAIL"
+	    rc=1
+	fi
+    done
+else
+    prlog " ... FAIL"
+    rc=1
+fi
+
+exit $rc

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
  2015-09-08 11:06 ` [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot Hiraku Toyooka
@ 2015-09-08 23:22     ` Mark Salyzyn
  2015-09-08 23:37     ` Kees Cook
  1 sibling, 0 replies; 28+ messages in thread
From: Mark Salyzyn @ 2015-09-08 23:22 UTC (permalink / raw)
  To: Hiraku Toyooka, linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Colin Cross, Seiji Aguchi

On 09/08/2015 04: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
>
> . . .
> +TEST_STRING="Testing pstore"
. . .
> +prlog -n "Checking pmsg file contains TEST_STRING ... "
> +grep -q "${TEST_STRING}" pmsg-${backend}-0
Mark this as 'wish to have'

Can TEST_STRING be given an unique value each run, so that on the the 
reboot-comparison run it can be found to be an unique match? Also 
confirm that any previous content (which may be binary) is not present 
after reboot, and that totally new content is present.

Sincerely -- Mark Salyzyn



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-08 23:22     ` Mark Salyzyn
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Salyzyn @ 2015-09-08 23:22 UTC (permalink / raw)
  To: Hiraku Toyooka, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Tony Luck, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Anton Vorontsov, Shuah Khan, Colin Cross, Seiji Aguchi

On 09/08/2015 04: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
>
> . . .
> +TEST_STRING="Testing pstore"
. . .
> +prlog -n "Checking pmsg file contains TEST_STRING ... "
> +grep -q "${TEST_STRING}" pmsg-${backend}-0
Mark this as 'wish to have'

Can TEST_STRING be given an unique value each run, so that on the the 
reboot-comparison run it can be found to be an unique match? Also 
confirm that any previous content (which may be binary) is not present 
after reboot, and that totally new content is present.

Sincerely -- Mark Salyzyn

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
  2015-09-08 11:06 ` [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot Hiraku Toyooka
@ 2015-09-08 23:37     ` Kees Cook
  2015-09-08 23:37     ` Kees Cook
  1 sibling, 0 replies; 28+ messages in thread
From: Kees Cook @ 2015-09-08 23:37 UTC (permalink / raw)
  To: Hiraku Toyooka
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
<hiraku.toyooka.gu@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.gu@hitachi.com>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Salyzyn <salyzyn@android.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-api@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 0000000..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
> +
> +clean:
> +       rm -rf logs/*
> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> new file mode 100755
> index 0000000..98611c5
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/common_tests
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +
> +# common_tests - Shell script commonly used by pstore test scripts
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> +#
> +# Released under the terms of the GPL v2.
> +
> +# Utilities
> +errexit() { # message
> +  echo "Error: $1" 1>&2
> +  exit 1
> +}
> +
> +absdir() { # file_path
> +  (cd `dirname $1`; pwd)
> +}
> +
> +# Parameters
> +TOP_DIR=`absdir $0`
> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
> +TEST_STRING="Testing pstore"
> +
> +# Preparing logs
> +LOG_FILE=$LOG_DIR/`basename $0`.log
> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
> +date > $LOG_FILE
> +prlog() { # messages
> +  /bin/echo "$@" | tee -a $LOG_FILE
> +}
> +prlog "=== Pstore unit tests (`basename $0`)==="
> +
> +# Starting tests
> +rc=0
> +
> +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
> +fi

This seems unstable if the system hasn't booted recently or if stuff
is spamming dmesg. What about examining /sys/module/pstore instead?

> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> new file mode 100755
> index 0000000..cbf613c
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/pstore_tests
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +# pstore_tests - Check pstore's behavior before crash/reboot
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> +#
> +# Released under the terms of the GPL v2.
> +
> +. ./common_tests
> +
> +prlog -n "Checking pstore console is registered ... "
> +dmesg | grep -q "console \[pstore"
> +if [ $? -eq 0 ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +fi
> +
> +prlog -n "Checking /dev/pmsg0 exists ... "
> +if [ -e "/dev/pmsg0" ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
> +if [ -e "/dev/pmsg0" ]; then
> +    echo "${TEST_STRING}" > /dev/pmsg0
> +    if [ $? -eq 0 ]; then
> +       prlog "ok"
> +    else
> +       prlog "FAIL"
> +       rc=1
> +    fi
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +exit $rc
>

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-08 23:37     ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2015-09-08 23:37 UTC (permalink / raw)
  To: Hiraku Toyooka
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
<hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org> 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.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
> Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Cc: Mark Salyzyn <salyzyn-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 0000000..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
> +
> +clean:
> +       rm -rf logs/*
> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> new file mode 100755
> index 0000000..98611c5
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/common_tests
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +
> +# common_tests - Shell script commonly used by pstore test scripts
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
> +#
> +# Released under the terms of the GPL v2.
> +
> +# Utilities
> +errexit() { # message
> +  echo "Error: $1" 1>&2
> +  exit 1
> +}
> +
> +absdir() { # file_path
> +  (cd `dirname $1`; pwd)
> +}
> +
> +# Parameters
> +TOP_DIR=`absdir $0`
> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
> +TEST_STRING="Testing pstore"
> +
> +# Preparing logs
> +LOG_FILE=$LOG_DIR/`basename $0`.log
> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
> +date > $LOG_FILE
> +prlog() { # messages
> +  /bin/echo "$@" | tee -a $LOG_FILE
> +}
> +prlog "=== Pstore unit tests (`basename $0`)==="
> +
> +# Starting tests
> +rc=0
> +
> +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
> +fi

This seems unstable if the system hasn't booted recently or if stuff
is spamming dmesg. What about examining /sys/module/pstore instead?

> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> new file mode 100755
> index 0000000..cbf613c
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/pstore_tests
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +# pstore_tests - Check pstore's behavior before crash/reboot
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
> +#
> +# Released under the terms of the GPL v2.
> +
> +. ./common_tests
> +
> +prlog -n "Checking pstore console is registered ... "
> +dmesg | grep -q "console \[pstore"
> +if [ $? -eq 0 ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +fi
> +
> +prlog -n "Checking /dev/pmsg0 exists ... "
> +if [ -e "/dev/pmsg0" ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
> +if [ -e "/dev/pmsg0" ]; then
> +    echo "${TEST_STRING}" > /dev/pmsg0
> +    if [ $? -eq 0 ]; then
> +       prlog "ok"
> +    else
> +       prlog "FAIL"
> +       rc=1
> +    fi
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +exit $rc
>

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
  2015-09-08 11:06   ` Hiraku Toyooka
  (?)
@ 2015-09-08 23:40   ` Kees Cook
  2015-09-15  2:41       ` Hiraku Toyooka
  -1 siblings, 1 reply; 28+ messages in thread
From: Kees Cook @ 2015-09-08 23:40 UTC (permalink / raw)
  To: Hiraku Toyooka
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
<hiraku.toyooka.gu@hitachi.com> wrote:
> To test pstore in earnest, we have to cause kernel crash and check
> pstore filesystem mouted after reboot.
>
> We add two scripts:
>  - pstore_crash_test
>      This script to cause crash and reboot easily. It is executed by
>      'make run_pstore_crash' in selftests.
>  - pstore_post_reboot_tests
>      This script includes test cases which check pstore's behavior after
>      crash and reboot. It is executed together with pstore_tests by
>      'make run_tests [-C pstore]' in selftests.
>
> The test cases in pstore_post_reboot_tests are currently following.
>
> - Check pstore backend is registered
> - Mount pstore filesystem
> - Check dmesg files exist in pstore filesystem
> - Check console file exist in pstore filesystem
> - Check pmsg file exist in pstore filesystem
> - Check dmesg files contain oops end marker
> - Check console file contain oops end marker
> - Check pmsg file contain the string written before crash
> - Remove all files in pstore filesystem
>
> Example usage is following.
>
> ...
> (kernel crash and reboot)
> ...
> 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
> Mounting pstore filesystem ... ok
> Checking dmesg files exist in pstore filesystem ... ok
>         dmesg-ramoops-0
>         dmesg-ramoops-1
> Checking console files exist in pstore filesystem ... ok
>         console-ramoops-0
> Checking pmsg files exist in pstore filesystem ... ok
>         pmsg-ramoops-0
> Checking dmesg files contains oops end marker
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
> Checking console file contains oops end marker ... ok
> Checking pmsg file contains TEST_STRING ... ok
> Removing all files in pstore filesystem
>         console-ramoops-0 ... ok
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
>         pmsg-ramoops-0 ... ok
> selftests: pstore_post_reboot_tests [PASS]
> make: Leaving directory '/home/root/selftests/pstore'
>
> Signed-off-by: Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Salyzyn <salyzyn@android.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-api@vger.kernel.org
> ---
>  tools/testing/selftests/pstore/Makefile            |    7 +
>  tools/testing/selftests/pstore/common_tests        |    1
>  tools/testing/selftests/pstore/pstore_crash_test   |   27 ++++
>  .../selftests/pstore/pstore_post_reboot_tests      |  126 ++++++++++++++++++++
>  4 files changed, 159 insertions(+), 2 deletions(-)
>  create mode 100755 tools/testing/selftests/pstore/pstore_crash_test
>  create mode 100755 tools/testing/selftests/pstore/pstore_post_reboot_tests
>
> diff --git a/tools/testing/selftests/pstore/Makefile b/tools/testing/selftests/pstore/Makefile
> index 40b887d..32c408c 100644
> --- a/tools/testing/selftests/pstore/Makefile
> +++ b/tools/testing/selftests/pstore/Makefile
> @@ -3,10 +3,13 @@
>
>  all:
>
> -TEST_PROGS := pstore_tests
> -TEST_FILES := common_tests
> +TEST_PROGS := pstore_tests pstore_post_reboot_tests
> +TEST_FILES := common_tests pstore_crash_test
>
>  include ../lib.mk
>
> +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; }

> +
>  clean:
>         rm -rf logs/*
> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> index 98611c5..8003760 100755
> --- a/tools/testing/selftests/pstore/common_tests
> +++ b/tools/testing/selftests/pstore/common_tests
> @@ -20,6 +20,7 @@ absdir() { # file_path
>  # Parameters
>  TOP_DIR=`absdir $0`
>  LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
> +REBOOT_FILE=$TOP_DIR/reboot_flag
>  TEST_STRING="Testing pstore"
>
>  # Preparing logs
> diff --git a/tools/testing/selftests/pstore/pstore_crash_test b/tools/testing/selftests/pstore/pstore_crash_test
> new file mode 100755
> index 0000000..6d0c422
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/pstore_crash_test
> @@ -0,0 +1,27 @@
> +#!/bin/sh
> +
> +# pstore_crash_test - Pstore test shell script which causes crash and reboot
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> +#
> +# Released under the terms of the GPL v2.
> +
> +# exit if pstore backend is not registered
> +. ./common_tests
> +
> +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
> diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> new file mode 100755
> index 0000000..0e33366
> --- /dev/null
> +++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> @@ -0,0 +1,126 @@
> +#!/bin/sh
> +
> +# pstore_post_reboot_tests - Check pstore's behavior after crash/reboot
> +#
> +# Copyright (C) Hitachi Ltd., 2015
> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> +#
> +# Released under the terms of the GPL v2.
> +
> +. ./common_tests
> +
> +if [ -e $REBOOT_FILE  ]; then
> +    rm $REBOOT_FILE
> +else
> +    prlog "pstore_crash_test has not been executed yet. we skip further tests."
> +    exit 0
> +fi
> +
> +prlog -n "Mounting pstore filesystem ... "
> +mount_info=`grep pstore /proc/mounts`
> +if [ $? -eq 0 ]; then
> +    mount_point=`echo ${mount_info} | cut -d' ' -f2 | head -n1`
> +    prlog "ok"
> +else
> +    mount none /sys/fs/pstore -t pstore
> +    if [ $? -eq 0 ]; then
> +       mount_point=`grep pstore /proc/mounts | cut -d' ' -f2 | head -n1`
> +       prlog "ok"
> +    else
> +       prlog "FAIL"
> +       exit 1
> +    fi
> +fi
> +
> +cd ${mount_point}
> +
> +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.

> +
> +prlog -n "Checking console files exist in pstore filesystem ... "
> +if [ -e console-${backend}-0 ]; then
> +    prlog "ok"
> +    for f in `ls console-${backend}-*`; do
> +       prlog -e "\t${f}"
> +    done
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Checking pmsg files exist in pstore filesystem ... "
> +if [ -e pmsg-${backend}-0 ]; then
> +    prlog "ok"
> +    for f in `ls pmsg-${backend}-*`; do
> +       prlog -e "\t${f}"
> +    done
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Checking dmesg files contains oops end marker"
> +files=`ls dmesg-${backend}-*`
> +if [ $? -eq 0 ]; then
> +    prlog
> +    for f in $files; do
> +       prlog -ne "\t${f} ... "
> +       grep -q "\---\[ end trace" $f
> +       if [ $? -eq 0 ]; then
> +           prlog "ok"
> +       else
> +           prlog "FAIL"
> +           rc=1
> +       fi
> +    done
> +else
> +    prlog " ... FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Checking console file contains oops end marker ... "
> +grep -q "\---\[ end trace" console-${backend}-0
> +if [ $? -eq 0 ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Checking pmsg file contains TEST_STRING ... "
> +grep -q "${TEST_STRING}" pmsg-${backend}-0
> +if [ $? -eq 0 ]; then
> +    prlog "ok"
> +else
> +    prlog "FAIL"
> +    rc=1
> +fi
> +
> +prlog -n "Removing all files in pstore filesystem "
> +files=`ls *-${backend}-*`
> +if [ $? -eq 0 ]; then
> +    prlog
> +    for f in ${files}; do
> +       prlog -ne "\t${f} ... "
> +       rm ${f}
> +       if [ $? -eq 0 ]; then
> +           prlog "ok"
> +       else
> +           prlog "FAIL"
> +           rc=1
> +       fi
> +    done
> +else
> +    prlog " ... FAIL"
> +    rc=1
> +fi
> +
> +exit $rc
>

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/2] selftests/pstore: add pstore test script
  2015-09-08 11:06 [PATCH 0/2] selftests/pstore: add pstore test script Hiraku Toyooka
@ 2015-09-08 23:42   ` Kees Cook
  2015-09-08 11:06   ` Hiraku Toyooka
  2015-09-08 23:42   ` Kees Cook
  2 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2015-09-08 23:42 UTC (permalink / raw)
  To: Hiraku Toyooka
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
<hiraku.toyooka.gu@hitachi.com> wrote:
> These scripts include test cases which check pstore behavior. This
> is useful to avoid regressions of pstore.
>
> Pstore is used across kernel crash, so these test cases are split
> into three parts.
>
>  - pstore_tests: check pstore behavior before crash
>  - pstore_post_reboot_tests: check pstore behavior after crash and reboot
>  - pstore_crash_test: cause kernel crash and reboot
>
> The pstore_test and the pstore_post_reboot_tests are the actual scripts
> for testing pstore and are executed in usual selftest's "run_test" target.
> On the other hand, the pstore_crash_test is to cause kernel panic and reboot,
> so it is executed in new "run_pstore_crash" target which is specified ad-hoc
> by users. In addition, there is a "common_tests" script which includes
> utilities and test cases used commonly in these scripts.
>
> When the pstore_crash_test is executed, it creates a file as a reboot flag.
> The pstore_post_reboot_tests detects whether the file exists or not. If the
> file doesn't exists, the test cases are skipped.
>
> These scripts expect that one pstore backend is registered before the
> scripts are executed.
> Assumed use case is following.
>
> # cd linux/tools/testing/selftests
> # make run_tests -C pstore
> 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'
> # make run_pstore_crash
> ...
> (kernel crash and reboot)
> ...
> # make run_tests -C pstore
> 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
> Mounting pstore filesystem ... ok
> Checking dmesg files exist in pstore filesystem ... ok
>         dmesg-ramoops-0
>         dmesg-ramoops-1
> Checking console files exist in pstore filesystem ... ok
>         console-ramoops-0
> Checking pmsg files exist in pstore filesystem ... ok
>         pmsg-ramoops-0
> Checking dmesg files contains oops end marker
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
> Checking console file contains oops end marker ... ok
> Checking pmsg file contains TEST_STRING ... ok
> Removing all files in pstore filesystem
>         console-ramoops-0 ... ok
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
>         pmsg-ramoops-0 ... ok
> selftests: pstore_post_reboot_tests [PASS]
> make: Leaving directory '/home/root/selftests/pstore'
>
>
> We can also see test logs later.
>
> # cat pstore/logs/20150903-111158/pstore_tests.log
> ...

Thanks for working on this! Pstore does need some selftests, so this
is greatly appreciated. :) I sent some improvement ideas in separate
emails.

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/2] selftests/pstore: add pstore test script
@ 2015-09-08 23:42   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2015-09-08 23:42 UTC (permalink / raw)
  To: Hiraku Toyooka
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, Seiji Aguchi

On Tue, Sep 8, 2015 at 4:06 AM, Hiraku Toyooka
<hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org> wrote:
> These scripts include test cases which check pstore behavior. This
> is useful to avoid regressions of pstore.
>
> Pstore is used across kernel crash, so these test cases are split
> into three parts.
>
>  - pstore_tests: check pstore behavior before crash
>  - pstore_post_reboot_tests: check pstore behavior after crash and reboot
>  - pstore_crash_test: cause kernel crash and reboot
>
> The pstore_test and the pstore_post_reboot_tests are the actual scripts
> for testing pstore and are executed in usual selftest's "run_test" target.
> On the other hand, the pstore_crash_test is to cause kernel panic and reboot,
> so it is executed in new "run_pstore_crash" target which is specified ad-hoc
> by users. In addition, there is a "common_tests" script which includes
> utilities and test cases used commonly in these scripts.
>
> When the pstore_crash_test is executed, it creates a file as a reboot flag.
> The pstore_post_reboot_tests detects whether the file exists or not. If the
> file doesn't exists, the test cases are skipped.
>
> These scripts expect that one pstore backend is registered before the
> scripts are executed.
> Assumed use case is following.
>
> # cd linux/tools/testing/selftests
> # make run_tests -C pstore
> 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'
> # make run_pstore_crash
> ...
> (kernel crash and reboot)
> ...
> # make run_tests -C pstore
> 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
> Mounting pstore filesystem ... ok
> Checking dmesg files exist in pstore filesystem ... ok
>         dmesg-ramoops-0
>         dmesg-ramoops-1
> Checking console files exist in pstore filesystem ... ok
>         console-ramoops-0
> Checking pmsg files exist in pstore filesystem ... ok
>         pmsg-ramoops-0
> Checking dmesg files contains oops end marker
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
> Checking console file contains oops end marker ... ok
> Checking pmsg file contains TEST_STRING ... ok
> Removing all files in pstore filesystem
>         console-ramoops-0 ... ok
>         dmesg-ramoops-0 ... ok
>         dmesg-ramoops-1 ... ok
>         pmsg-ramoops-0 ... ok
> selftests: pstore_post_reboot_tests [PASS]
> make: Leaving directory '/home/root/selftests/pstore'
>
>
> We can also see test logs later.
>
> # cat pstore/logs/20150903-111158/pstore_tests.log
> ...

Thanks for working on this! Pstore does need some selftests, so this
is greatly appreciated. :) I sent some improvement ideas in separate
emails.

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-15  2:30       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:30 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Colin Cross, seiji.aguchi.tr

Hello Mark,

Thank you for your advise.

 >> +prlog -n "Checking pmsg file contains TEST_STRING ... "
 > Mark this as 'wish to have'

OK. I'll change it to "Checking pmsg file wishes to have TEST_STRING
... ". Should I change other messages in the same way?

 > Can TEST_STRING be given an unique value each run, so that on the the
 > reboot-comparison run it can be found to be an unique match?

Yes. I'll append /proc/sys/kernel/random/uuid content to TEST_STRING.
I'll also change log directory name from date to the uuid.

 > Also confirm that any previous content (which may be binary) is not
 > present after reboot, and that totally new content is present.

OK.
As for pmsg, they are possible by checking if the /sys/fs/pstore/pmsg
content perfectly matches the TEST_STRING which was written to /dev/pmsg
before reboot. (The TEST_STRING can be left to a regular file before
reboot as well as reboot_flag.) Is it OK?

Best regards,
Hiraku Toyooka


-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-15  2:30       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:30 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Tony Luck, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Anton Vorontsov, Shuah Khan, Colin Cross,
	seiji.aguchi.tr-FCd8Q96Dh0JBDgjK7y7TUQ

Hello Mark,

Thank you for your advise.

 >> +prlog -n "Checking pmsg file contains TEST_STRING ... "
 > Mark this as 'wish to have'

OK. I'll change it to "Checking pmsg file wishes to have TEST_STRING
... ". Should I change other messages in the same way?

 > Can TEST_STRING be given an unique value each run, so that on the the
 > reboot-comparison run it can be found to be an unique match?

Yes. I'll append /proc/sys/kernel/random/uuid content to TEST_STRING.
I'll also change log directory name from date to the uuid.

 > Also confirm that any previous content (which may be binary) is not
 > present after reboot, and that totally new content is present.

OK.
As for pmsg, they are possible by checking if the /sys/fs/pstore/pmsg
content perfectly matches the TEST_STRING which was written to /dev/pmsg
before reboot. (The TEST_STRING can be left to a regular file before
reboot as well as reboot_flag.) Is it OK?

Best regards,
Hiraku Toyooka


-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-15  2:31       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:31 UTC (permalink / raw)
  To: Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, seiji.aguchi.tr

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.gu@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.gu@hitachi.com>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Anton Vorontsov <anton@enomsg.org>
>> Cc: Colin Cross <ccross@android.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Mark Salyzyn <salyzyn@android.com>
>> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-api@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 0000000..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
>> +
>> +clean:
>> +       rm -rf logs/*
>> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
>> new file mode 100755
>> index 0000000..98611c5
>> --- /dev/null
>> +++ b/tools/testing/selftests/pstore/common_tests
>> @@ -0,0 +1,45 @@
>> +#!/bin/sh
>> +
>> +# common_tests - Shell script commonly used by pstore test scripts
>> +#
>> +# Copyright (C) Hitachi Ltd., 2015
>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
>> +#
>> +# Released under the terms of the GPL v2.
>> +
>> +# Utilities
>> +errexit() { # message
>> +  echo "Error: $1" 1>&2
>> +  exit 1
>> +}
>> +
>> +absdir() { # file_path
>> +  (cd `dirname $1`; pwd)
>> +}
>> +
>> +# Parameters
>> +TOP_DIR=`absdir $0`
>> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
>> +TEST_STRING="Testing pstore"
>> +
>> +# Preparing logs
>> +LOG_FILE=$LOG_DIR/`basename $0`.log
>> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
>> +date > $LOG_FILE
>> +prlog() { # messages
>> +  /bin/echo "$@" | tee -a $LOG_FILE
>> +}
>> +prlog "=== Pstore unit tests (`basename $0`)==="
>> +
>> +# Starting tests
>> +rc=0
>> +
>> +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
>> +fi
>
> This seems unstable if the system hasn't booted recently or if stuff
> is spamming dmesg. What about examining /sys/module/pstore instead?
>
>> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
>> new file mode 100755
>> index 0000000..cbf613c
>> --- /dev/null
>> +++ b/tools/testing/selftests/pstore/pstore_tests
>> @@ -0,0 +1,42 @@
>> +#!/bin/sh
>> +
>> +# pstore_tests - Check pstore's behavior before crash/reboot
>> +#
>> +# Copyright (C) Hitachi Ltd., 2015
>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
>> +#
>> +# Released under the terms of the GPL v2.
>> +
>> +. ./common_tests
>> +
>> +prlog -n "Checking pstore console is registered ... "
>> +dmesg | grep -q "console \[pstore"
>> +if [ $? -eq 0 ]; then
>> +    prlog "ok"
>> +else
>> +    prlog "FAIL"
>> +fi
>> +
>> +prlog -n "Checking /dev/pmsg0 exists ... "
>> +if [ -e "/dev/pmsg0" ]; then
>> +    prlog "ok"
>> +else
>> +    prlog "FAIL"
>> +    rc=1
>> +fi
>> +
>> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
>> +if [ -e "/dev/pmsg0" ]; then
>> +    echo "${TEST_STRING}" > /dev/pmsg0
>> +    if [ $? -eq 0 ]; then
>> +       prlog "ok"
>> +    else
>> +       prlog "FAIL"
>> +       rc=1
>> +    fi
>> +else
>> +    prlog "FAIL"
>> +    rc=1
>> +fi
>> +
>> +exit $rc
>>
>
> -Kees
>

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-15  2:31       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:31 UTC (permalink / raw)
  To: Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross,
	seiji.aguchi.tr-FCd8Q96Dh0JBDgjK7y7TUQ

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.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org> 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.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>> Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
>> Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Cc: Mark Salyzyn <salyzyn-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>> Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 0000000..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
>> +
>> +clean:
>> +       rm -rf logs/*
>> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
>> new file mode 100755
>> index 0000000..98611c5
>> --- /dev/null
>> +++ b/tools/testing/selftests/pstore/common_tests
>> @@ -0,0 +1,45 @@
>> +#!/bin/sh
>> +
>> +# common_tests - Shell script commonly used by pstore test scripts
>> +#
>> +# Copyright (C) Hitachi Ltd., 2015
>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>> +#
>> +# Released under the terms of the GPL v2.
>> +
>> +# Utilities
>> +errexit() { # message
>> +  echo "Error: $1" 1>&2
>> +  exit 1
>> +}
>> +
>> +absdir() { # file_path
>> +  (cd `dirname $1`; pwd)
>> +}
>> +
>> +# Parameters
>> +TOP_DIR=`absdir $0`
>> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
>> +TEST_STRING="Testing pstore"
>> +
>> +# Preparing logs
>> +LOG_FILE=$LOG_DIR/`basename $0`.log
>> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
>> +date > $LOG_FILE
>> +prlog() { # messages
>> +  /bin/echo "$@" | tee -a $LOG_FILE
>> +}
>> +prlog "=== Pstore unit tests (`basename $0`)==="
>> +
>> +# Starting tests
>> +rc=0
>> +
>> +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
>> +fi
>
> This seems unstable if the system hasn't booted recently or if stuff
> is spamming dmesg. What about examining /sys/module/pstore instead?
>
>> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
>> new file mode 100755
>> index 0000000..cbf613c
>> --- /dev/null
>> +++ b/tools/testing/selftests/pstore/pstore_tests
>> @@ -0,0 +1,42 @@
>> +#!/bin/sh
>> +
>> +# pstore_tests - Check pstore's behavior before crash/reboot
>> +#
>> +# Copyright (C) Hitachi Ltd., 2015
>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>> +#
>> +# Released under the terms of the GPL v2.
>> +
>> +. ./common_tests
>> +
>> +prlog -n "Checking pstore console is registered ... "
>> +dmesg | grep -q "console \[pstore"
>> +if [ $? -eq 0 ]; then
>> +    prlog "ok"
>> +else
>> +    prlog "FAIL"
>> +fi
>> +
>> +prlog -n "Checking /dev/pmsg0 exists ... "
>> +if [ -e "/dev/pmsg0" ]; then
>> +    prlog "ok"
>> +else
>> +    prlog "FAIL"
>> +    rc=1
>> +fi
>> +
>> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
>> +if [ -e "/dev/pmsg0" ]; then
>> +    echo "${TEST_STRING}" > /dev/pmsg0
>> +    if [ $? -eq 0 ]; then
>> +       prlog "ok"
>> +    else
>> +       prlog "FAIL"
>> +       rc=1
>> +    fi
>> +else
>> +    prlog "FAIL"
>> +    rc=1
>> +fi
>> +
>> +exit $rc
>>
>
> -Kees
>

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-15  2:41       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:41 UTC (permalink / raw)
  To: Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross, seiji.aguchi.tr

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-15  2:41       ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-15  2:41 UTC (permalink / raw)
  To: Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross,
	seiji.aguchi.tr-FCd8Q96Dh0JBDgjK7y7TUQ

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-16 12:02         ` 阿口誠司 / AGUCHI,SEIJI
  0 siblings, 0 replies; 28+ messages in thread
From: 阿口誠司 / AGUCHI,SEIJI @ 2015-09-16 12:02 UTC (permalink / raw)
  To: 豊岡拓 / Toyooka,Hiraku, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 7579 bytes --]

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.gu@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.gu@hitachi.com>
> >> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> >> Cc: Tony Luck <tony.luck@intel.com>
> >> Cc: Anton Vorontsov <anton@enomsg.org>
> >> Cc: Colin Cross <ccross@android.com>
> >> Cc: Kees Cook <keescook@chromium.org>
> >> Cc: Mark Salyzyn <salyzyn@android.com>
> >> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-api@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 0000000..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
> >> +
> >> +clean:
> >> +       rm -rf logs/*
> >> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> >> new file mode 100755
> >> index 0000000..98611c5
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/common_tests
> >> @@ -0,0 +1,45 @@
> >> +#!/bin/sh
> >> +
> >> +# common_tests - Shell script commonly used by pstore test scripts
> >> +#
> >> +# Copyright (C) Hitachi Ltd., 2015
> >> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> >> +#
> >> +# Released under the terms of the GPL v2.
> >> +
> >> +# Utilities
> >> +errexit() { # message
> >> +  echo "Error: $1" 1>&2
> >> +  exit 1
> >> +}
> >> +
> >> +absdir() { # file_path
> >> +  (cd `dirname $1`; pwd)
> >> +}
> >> +
> >> +# Parameters
> >> +TOP_DIR=`absdir $0`
> >> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
> >> +TEST_STRING="Testing pstore"
> >> +
> >> +# Preparing logs
> >> +LOG_FILE=$LOG_DIR/`basename $0`.log
> >> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
> >> +date > $LOG_FILE
> >> +prlog() { # messages
> >> +  /bin/echo "$@" | tee -a $LOG_FILE
> >> +}
> >> +prlog "=== Pstore unit tests (`basename $0`)==="
> >> +
> >> +# Starting tests
> >> +rc=0
> >> +
> >> +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
> >> +fi
> >
> > This seems unstable if the system hasn't booted recently or if stuff
> > is spamming dmesg. What about examining /sys/module/pstore instead?
> >
> >> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> >> new file mode 100755
> >> index 0000000..cbf613c
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/pstore_tests
> >> @@ -0,0 +1,42 @@
> >> +#!/bin/sh
> >> +
> >> +# pstore_tests - Check pstore's behavior before crash/reboot
> >> +#
> >> +# Copyright (C) Hitachi Ltd., 2015
> >> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> >> +#
> >> +# Released under the terms of the GPL v2.
> >> +
> >> +. ./common_tests
> >> +
> >> +prlog -n "Checking pstore console is registered ... "
> >> +dmesg | grep -q "console \[pstore"
> >> +if [ $? -eq 0 ]; then
> >> +    prlog "ok"
> >> +else
> >> +    prlog "FAIL"
> >> +fi
> >> +
> >> +prlog -n "Checking /dev/pmsg0 exists ... "
> >> +if [ -e "/dev/pmsg0" ]; then
> >> +    prlog "ok"
> >> +else
> >> +    prlog "FAIL"
> >> +    rc=1
> >> +fi
> >> +
> >> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
> >> +if [ -e "/dev/pmsg0" ]; then
> >> +    echo "${TEST_STRING}" > /dev/pmsg0
> >> +    if [ $? -eq 0 ]; then
> >> +       prlog "ok"
> >> +    else
> >> +       prlog "FAIL"
> >> +       rc=1
> >> +    fi
> >> +else
> >> +    prlog "FAIL"
> >> +    rc=1
> >> +fi
> >> +
> >> +exit $rc
> >>
> >
> > -Kees
> >
> 
> --
> Hiraku Toyooka
> Systems Productivity Research Dept. / Linux Technology Center
> Center for Technology Innovation - Systems Engineering, Hitachi Ltd.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-16 12:02         ` 阿口誠司 / AGUCHI,SEIJI
  0 siblings, 0 replies; 28+ messages in thread
From: 阿口誠司 / AGUCHI,SEIJI @ 2015-09-16 12:02 UTC (permalink / raw)
  To: 豊岡拓 / Toyooka,Hiraku, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

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.gu@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.gu@hitachi.com>
> >> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> >> Cc: Tony Luck <tony.luck@intel.com>
> >> Cc: Anton Vorontsov <anton@enomsg.org>
> >> Cc: Colin Cross <ccross@android.com>
> >> Cc: Kees Cook <keescook@chromium.org>
> >> Cc: Mark Salyzyn <salyzyn@android.com>
> >> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-api@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 0000000..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
> >> +
> >> +clean:
> >> +       rm -rf logs/*
> >> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> >> new file mode 100755
> >> index 0000000..98611c5
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/common_tests
> >> @@ -0,0 +1,45 @@
> >> +#!/bin/sh
> >> +
> >> +# common_tests - Shell script commonly used by pstore test scripts
> >> +#
> >> +# Copyright (C) Hitachi Ltd., 2015
> >> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> >> +#
> >> +# Released under the terms of the GPL v2.
> >> +
> >> +# Utilities
> >> +errexit() { # message
> >> +  echo "Error: $1" 1>&2
> >> +  exit 1
> >> +}
> >> +
> >> +absdir() { # file_path
> >> +  (cd `dirname $1`; pwd)
> >> +}
> >> +
> >> +# Parameters
> >> +TOP_DIR=`absdir $0`
> >> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
> >> +TEST_STRING="Testing pstore"
> >> +
> >> +# Preparing logs
> >> +LOG_FILE=$LOG_DIR/`basename $0`.log
> >> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
> >> +date > $LOG_FILE
> >> +prlog() { # messages
> >> +  /bin/echo "$@" | tee -a $LOG_FILE
> >> +}
> >> +prlog "=== Pstore unit tests (`basename $0`)==="
> >> +
> >> +# Starting tests
> >> +rc=0
> >> +
> >> +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
> >> +fi
> >
> > This seems unstable if the system hasn't booted recently or if stuff
> > is spamming dmesg. What about examining /sys/module/pstore instead?
> >
> >> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> >> new file mode 100755
> >> index 0000000..cbf613c
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/pstore/pstore_tests
> >> @@ -0,0 +1,42 @@
> >> +#!/bin/sh
> >> +
> >> +# pstore_tests - Check pstore's behavior before crash/reboot
> >> +#
> >> +# Copyright (C) Hitachi Ltd., 2015
> >> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
> >> +#
> >> +# Released under the terms of the GPL v2.
> >> +
> >> +. ./common_tests
> >> +
> >> +prlog -n "Checking pstore console is registered ... "
> >> +dmesg | grep -q "console \[pstore"
> >> +if [ $? -eq 0 ]; then
> >> +    prlog "ok"
> >> +else
> >> +    prlog "FAIL"
> >> +fi
> >> +
> >> +prlog -n "Checking /dev/pmsg0 exists ... "
> >> +if [ -e "/dev/pmsg0" ]; then
> >> +    prlog "ok"
> >> +else
> >> +    prlog "FAIL"
> >> +    rc=1
> >> +fi
> >> +
> >> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
> >> +if [ -e "/dev/pmsg0" ]; then
> >> +    echo "${TEST_STRING}" > /dev/pmsg0
> >> +    if [ $? -eq 0 ]; then
> >> +       prlog "ok"
> >> +    else
> >> +       prlog "FAIL"
> >> +       rc=1
> >> +    fi
> >> +else
> >> +    prlog "FAIL"
> >> +    rc=1
> >> +fi
> >> +
> >> +exit $rc
> >>
> >
> > -Kees
> >
> 
> --
> Hiraku Toyooka
> Systems Productivity Research Dept. / Linux Technology Center
> Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
  2015-09-15  2:41       ` Hiraku Toyooka
@ 2015-09-16 12:11         ` 阿口誠司 / AGUCHI,SEIJI
  -1 siblings, 0 replies; 28+ messages in thread
From: 阿口誠司 / AGUCHI,SEIJI @ 2015-09-16 12:11 UTC (permalink / raw)
  To: 豊岡拓 / Toyooka,Hiraku, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1997 bytes --]


>+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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-16 12:11         ` 阿口誠司 / AGUCHI,SEIJI
  0 siblings, 0 replies; 28+ messages in thread
From: 阿口誠司 / AGUCHI,SEIJI @ 2015-09-16 12:11 UTC (permalink / raw)
  To: 豊岡拓 / Toyooka,Hiraku, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross


>+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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-17  5:54           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-17  5:54 UTC (permalink / raw)
  To: 阿口誠司 / AGUCHI,SEIJI, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

Hello,

 > It may be good if you can log  "/sys/module/pstore/parameters/backend/"
 > or /proc/cmdline in failure case.
 >
 > It makes debug easy.

OK, I'll have the script log the information in v2.

Best regards,
Hiraku Toyooka

阿口誠司 / AGUCHI,SEIJI wrote:
> 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.gu@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.gu@hitachi.com>
>>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>>> Cc: Tony Luck <tony.luck@intel.com>
>>>> Cc: Anton Vorontsov <anton@enomsg.org>
>>>> Cc: Colin Cross <ccross@android.com>
>>>> Cc: Kees Cook <keescook@chromium.org>
>>>> Cc: Mark Salyzyn <salyzyn@android.com>
>>>> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: linux-api@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 0000000..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
>>>> +
>>>> +clean:
>>>> +       rm -rf logs/*
>>>> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
>>>> new file mode 100755
>>>> index 0000000..98611c5
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/pstore/common_tests
>>>> @@ -0,0 +1,45 @@
>>>> +#!/bin/sh
>>>> +
>>>> +# common_tests - Shell script commonly used by pstore test scripts
>>>> +#
>>>> +# Copyright (C) Hitachi Ltd., 2015
>>>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
>>>> +#
>>>> +# Released under the terms of the GPL v2.
>>>> +
>>>> +# Utilities
>>>> +errexit() { # message
>>>> +  echo "Error: $1" 1>&2
>>>> +  exit 1
>>>> +}
>>>> +
>>>> +absdir() { # file_path
>>>> +  (cd `dirname $1`; pwd)
>>>> +}
>>>> +
>>>> +# Parameters
>>>> +TOP_DIR=`absdir $0`
>>>> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
>>>> +TEST_STRING="Testing pstore"
>>>> +
>>>> +# Preparing logs
>>>> +LOG_FILE=$LOG_DIR/`basename $0`.log
>>>> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
>>>> +date > $LOG_FILE
>>>> +prlog() { # messages
>>>> +  /bin/echo "$@" | tee -a $LOG_FILE
>>>> +}
>>>> +prlog "=== Pstore unit tests (`basename $0`)==="
>>>> +
>>>> +# Starting tests
>>>> +rc=0
>>>> +
>>>> +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
>>>> +fi
>>>
>>> This seems unstable if the system hasn't booted recently or if stuff
>>> is spamming dmesg. What about examining /sys/module/pstore instead?
>>>
>>>> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
>>>> new file mode 100755
>>>> index 0000000..cbf613c
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/pstore/pstore_tests
>>>> @@ -0,0 +1,42 @@
>>>> +#!/bin/sh
>>>> +
>>>> +# pstore_tests - Check pstore's behavior before crash/reboot
>>>> +#
>>>> +# Copyright (C) Hitachi Ltd., 2015
>>>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com>
>>>> +#
>>>> +# Released under the terms of the GPL v2.
>>>> +
>>>> +. ./common_tests
>>>> +
>>>> +prlog -n "Checking pstore console is registered ... "
>>>> +dmesg | grep -q "console \[pstore"
>>>> +if [ $? -eq 0 ]; then
>>>> +    prlog "ok"
>>>> +else
>>>> +    prlog "FAIL"
>>>> +fi
>>>> +
>>>> +prlog -n "Checking /dev/pmsg0 exists ... "
>>>> +if [ -e "/dev/pmsg0" ]; then
>>>> +    prlog "ok"
>>>> +else
>>>> +    prlog "FAIL"
>>>> +    rc=1
>>>> +fi
>>>> +
>>>> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
>>>> +if [ -e "/dev/pmsg0" ]; then
>>>> +    echo "${TEST_STRING}" > /dev/pmsg0
>>>> +    if [ $? -eq 0 ]; then
>>>> +       prlog "ok"
>>>> +    else
>>>> +       prlog "FAIL"
>>>> +       rc=1
>>>> +    fi
>>>> +else
>>>> +    prlog "FAIL"
>>>> +    rc=1
>>>> +fi
>>>> +
>>>> +exit $rc
>>>>
>>>
>>> -Kees
>>>
>>
>> --
>> Hiraku Toyooka
>> Systems Productivity Research Dept. / Linux Technology Center
>> Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-17  5:54           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-17  5:54 UTC (permalink / raw)
  To: 阿口誠司 / AGUCHI,SEIJI, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

Hello,

 > It may be good if you can log  "/sys/module/pstore/parameters/backend/"
 > or /proc/cmdline in failure case.
 >
 > It makes debug easy.

OK, I'll have the script log the information in v2.

Best regards,
Hiraku Toyooka

阿口誠司 / AGUCHI,SEIJI wrote:
> 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.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org> 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.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>>> Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>>> Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
>>>> Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>>> Cc: Mark Salyzyn <salyzyn-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
>>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 0000000..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
>>>> +
>>>> +clean:
>>>> +       rm -rf logs/*
>>>> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
>>>> new file mode 100755
>>>> index 0000000..98611c5
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/pstore/common_tests
>>>> @@ -0,0 +1,45 @@
>>>> +#!/bin/sh
>>>> +
>>>> +# common_tests - Shell script commonly used by pstore test scripts
>>>> +#
>>>> +# Copyright (C) Hitachi Ltd., 2015
>>>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>>>> +#
>>>> +# Released under the terms of the GPL v2.
>>>> +
>>>> +# Utilities
>>>> +errexit() { # message
>>>> +  echo "Error: $1" 1>&2
>>>> +  exit 1
>>>> +}
>>>> +
>>>> +absdir() { # file_path
>>>> +  (cd `dirname $1`; pwd)
>>>> +}
>>>> +
>>>> +# Parameters
>>>> +TOP_DIR=`absdir $0`
>>>> +LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/
>>>> +TEST_STRING="Testing pstore"
>>>> +
>>>> +# Preparing logs
>>>> +LOG_FILE=$LOG_DIR/`basename $0`.log
>>>> +mkdir -p $LOG_DIR || errexit "Failed to make a log directory: $LOG_DIR"
>>>> +date > $LOG_FILE
>>>> +prlog() { # messages
>>>> +  /bin/echo "$@" | tee -a $LOG_FILE
>>>> +}
>>>> +prlog "=== Pstore unit tests (`basename $0`)==="
>>>> +
>>>> +# Starting tests
>>>> +rc=0
>>>> +
>>>> +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
>>>> +fi
>>>
>>> This seems unstable if the system hasn't booted recently or if stuff
>>> is spamming dmesg. What about examining /sys/module/pstore instead?
>>>
>>>> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
>>>> new file mode 100755
>>>> index 0000000..cbf613c
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/pstore/pstore_tests
>>>> @@ -0,0 +1,42 @@
>>>> +#!/bin/sh
>>>> +
>>>> +# pstore_tests - Check pstore's behavior before crash/reboot
>>>> +#
>>>> +# Copyright (C) Hitachi Ltd., 2015
>>>> +#  Written by Hiraku Toyooka <hiraku.toyooka.gu-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>
>>>> +#
>>>> +# Released under the terms of the GPL v2.
>>>> +
>>>> +. ./common_tests
>>>> +
>>>> +prlog -n "Checking pstore console is registered ... "
>>>> +dmesg | grep -q "console \[pstore"
>>>> +if [ $? -eq 0 ]; then
>>>> +    prlog "ok"
>>>> +else
>>>> +    prlog "FAIL"
>>>> +fi
>>>> +
>>>> +prlog -n "Checking /dev/pmsg0 exists ... "
>>>> +if [ -e "/dev/pmsg0" ]; then
>>>> +    prlog "ok"
>>>> +else
>>>> +    prlog "FAIL"
>>>> +    rc=1
>>>> +fi
>>>> +
>>>> +prlog -n "Writing TEST_STRING to /dev/pmsg0 ... "
>>>> +if [ -e "/dev/pmsg0" ]; then
>>>> +    echo "${TEST_STRING}" > /dev/pmsg0
>>>> +    if [ $? -eq 0 ]; then
>>>> +       prlog "ok"
>>>> +    else
>>>> +       prlog "FAIL"
>>>> +       rc=1
>>>> +    fi
>>>> +else
>>>> +    prlog "FAIL"
>>>> +    rc=1
>>>> +fi
>>>> +
>>>> +exit $rc
>>>>
>>>
>>> -Kees
>>>
>>
>> --
>> Hiraku Toyooka
>> Systems Productivity Research Dept. / Linux Technology Center
>> Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-17  5:54           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-17  5:54 UTC (permalink / raw)
  To: 阿口誠司 / AGUCHI,SEIJI, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

Hello,

 >> +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?

Yes, I should check /sys/kernel/kexec_crash_loaded. If the value is
1, this script should try to unload kexec kernel.

 > Or, does it cover oops and kdump case?

No, not yet. I think we should support oops case at first.

Best regards,
Hiraku Toyooka

阿口誠司 / AGUCHI,SEIJI wrote:
>
>> +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

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot
@ 2015-09-17  5:54           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-17  5:54 UTC (permalink / raw)
  To: 阿口誠司 / AGUCHI,SEIJI, Kees Cook
  Cc: LKML, Tony Luck, Linux API, Anton Vorontsov, Shuah Khan,
	Mark Salyzyn, Colin Cross

Hello,

 >> +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?

Yes, I should check /sys/kernel/kexec_crash_loaded. If the value is
1, this script should try to unload kexec kernel.

 > Or, does it cover oops and kdump case?

No, not yet. I think we should support oops case at first.

Best regards,
Hiraku Toyooka

阿口誠司 / AGUCHI,SEIJI wrote:
>
>> +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

-- 
Hiraku Toyooka
Systems Productivity Research Dept. / Linux Technology Center
Center for Technology Innovation - Systems Engineering, Hitachi Ltd.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
  2015-09-15  2:30       ` Hiraku Toyooka
  (?)
@ 2015-09-21 21:04       ` Mark Salyzyn
  2015-09-29  7:18           ` Hiraku Toyooka
  -1 siblings, 1 reply; 28+ messages in thread
From: Mark Salyzyn @ 2015-09-21 21:04 UTC (permalink / raw)
  To: Hiraku Toyooka, linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Colin Cross, seiji.aguchi.tr

On 09/14/2015 07:30 PM, Hiraku Toyooka wrote:
> Hello Mark,
>
> Thank you for your advise.
>
> >> +prlog -n "Checking pmsg file contains TEST_STRING ... "
> > Mark this as 'wish to have'
>
> OK. I'll change it to "Checking pmsg file wishes to have TEST_STRING
> ... ". Should I change other messages in the same way?

I was referring to the list of enhancements to the testing (below), not 
the message in the logs ;-}

Sorry for taking so long to respond, got preoccupied ...
>
> > Can TEST_STRING be given an unique value each run, so that on the the
> > reboot-comparison run it can be found to be an unique match?
>
> Yes. I'll append /proc/sys/kernel/random/uuid content to TEST_STRING.
> I'll also change log directory name from date to the uuid.

Cool

>
> > Also confirm that any previous content (which may be binary) is not
> > present after reboot, and that totally new content is present.
>
> OK.
> As for pmsg, they are possible by checking if the /sys/fs/pstore/pmsg
> content perfectly matches the TEST_STRING which was written to /dev/pmsg
> before reboot. (The TEST_STRING can be left to a regular file before
> reboot as well as reboot_flag.) Is it OK?

Perfectly match is an issue, since something else might be using pmsg. 
For instance, one of the applications that uses this interface 
packetizes the messages so they can be picked out from other sources 
that do not comply with the header (count, magic number etc). In this 
case, should that daemon be active, your content would be ignores, but 
your content would also be buried, but can be needled out with grep.

What you should do is grep for your string pattern within some 
acceptable regex, and one should be found and no other, and it should 
match perfectly. This would prevent another daemon's content from 
disrupting your test and causing a false negative.
>
> Best regards,
> Hiraku Toyooka
>
Sincerely -- Mark Salyzyn


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-29  7:18           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-29  7:18 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel
  Cc: Tony Luck, Kees Cook, linux-api, Anton Vorontsov, Shuah Khan,
	Colin Cross, seiji.aguchi.tr

Hello,

I'm sorry for my late reply.

Mark Salyzyn wrote:
> Perfectly match is an issue, since something else might be using pmsg.
> For instance, one of the applications that uses this interface
> packetizes the messages so they can be picked out from other sources
> that do not comply with the header (count, magic number etc). In this
> case, should that daemon be active, your content would be ignores, but
> your content would also be buried, but can be needled out with grep.
>
> What you should do is grep for your string pattern within some
> acceptable regex, and one should be found and no other, and it should
> match perfectly. This would prevent another daemon's content from
> disrupting your test and causing a false negative.

OK. I think that the following method suit your intention.
By splitting unique test string into TEST_STRING_PATTERN part and UUID
part, we can check both the non-existence of previous content and the
unique match on reboot-comparison run.
I'll include this in v2.

# before crash
TEST_STRING_PATTERN="Testing pstore: uuid="
UUID=`cat /proc/sys/kernel/random/uuid`
echo "$TEST_STRING_PATTERN""$UUID" > /dev/pmsg0
echo "$UUID" > uuid

# after crash
prlog -n "Checking pmsg file properly keeps the content written before 
crash ... "
nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
if [ $nr_matched -eq 1 ]; then
     grep -q "$TEST_STRING_PATTERN"`cat uuid` pmsg-${backend}-0
     if [ $? -eq 0 ]; then
         prlog "ok"
     else
         prlog "FAIL"
else
     prlog "FAIL"
fi


Best regards,
Hiraku Toyooka


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot
@ 2015-09-29  7:18           ` Hiraku Toyooka
  0 siblings, 0 replies; 28+ messages in thread
From: Hiraku Toyooka @ 2015-09-29  7:18 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Tony Luck, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Anton Vorontsov, Shuah Khan, Colin Cross,
	seiji.aguchi.tr-FCd8Q96Dh0JBDgjK7y7TUQ

Hello,

I'm sorry for my late reply.

Mark Salyzyn wrote:
> Perfectly match is an issue, since something else might be using pmsg.
> For instance, one of the applications that uses this interface
> packetizes the messages so they can be picked out from other sources
> that do not comply with the header (count, magic number etc). In this
> case, should that daemon be active, your content would be ignores, but
> your content would also be buried, but can be needled out with grep.
>
> What you should do is grep for your string pattern within some
> acceptable regex, and one should be found and no other, and it should
> match perfectly. This would prevent another daemon's content from
> disrupting your test and causing a false negative.

OK. I think that the following method suit your intention.
By splitting unique test string into TEST_STRING_PATTERN part and UUID
part, we can check both the non-existence of previous content and the
unique match on reboot-comparison run.
I'll include this in v2.

# before crash
TEST_STRING_PATTERN="Testing pstore: uuid="
UUID=`cat /proc/sys/kernel/random/uuid`
echo "$TEST_STRING_PATTERN""$UUID" > /dev/pmsg0
echo "$UUID" > uuid

# after crash
prlog -n "Checking pmsg file properly keeps the content written before 
crash ... "
nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
if [ $nr_matched -eq 1 ]; then
     grep -q "$TEST_STRING_PATTERN"`cat uuid` pmsg-${backend}-0
     if [ $? -eq 0 ]; then
         prlog "ok"
     else
         prlog "FAIL"
else
     prlog "FAIL"
fi


Best regards,
Hiraku Toyooka

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2015-09-29  7:19 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-08 11:06 [PATCH 0/2] selftests/pstore: add pstore test script Hiraku Toyooka
2015-09-08 11:06 ` [PATCH 1/2] selftests/pstore: add pstore test script for pre-reboot Hiraku Toyooka
2015-09-08 23:22   ` Mark Salyzyn
2015-09-08 23:22     ` Mark Salyzyn
2015-09-15  2:30     ` Hiraku Toyooka
2015-09-15  2:30       ` Hiraku Toyooka
2015-09-21 21:04       ` Mark Salyzyn
2015-09-29  7:18         ` Hiraku Toyooka
2015-09-29  7:18           ` Hiraku Toyooka
2015-09-08 23:37   ` Kees Cook
2015-09-08 23:37     ` Kees Cook
2015-09-15  2:31     ` Hiraku Toyooka
2015-09-15  2:31       ` Hiraku Toyooka
2015-09-16 12:02       ` 阿口誠司 / AGUCHI,SEIJI
2015-09-16 12:02         ` 阿口誠司 / AGUCHI,SEIJI
2015-09-17  5:54         ` Hiraku Toyooka
2015-09-17  5:54           ` Hiraku Toyooka
2015-09-08 11:06 ` [PATCH 2/2] selftests/pstore: add pstore test scripts going with reboot Hiraku Toyooka
2015-09-08 11:06   ` Hiraku Toyooka
2015-09-08 23:40   ` Kees Cook
2015-09-15  2:41     ` Hiraku Toyooka
2015-09-15  2:41       ` Hiraku Toyooka
2015-09-16 12:11       ` 阿口誠司 / AGUCHI,SEIJI
2015-09-16 12:11         ` 阿口誠司 / AGUCHI,SEIJI
2015-09-17  5:54         ` Hiraku Toyooka
2015-09-17  5:54           ` Hiraku Toyooka
2015-09-08 23:42 ` [PATCH 0/2] selftests/pstore: add pstore test script Kees Cook
2015-09-08 23:42   ` Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.