I have been steadily working but struggled to find a seamlessly
integrated way to implement tty frontend until Guilherme inspired me
that multi-backend and tty frontend are actually two separate entities.
This submission presents the 3rd iteration of my efforts, listing
notable changes form the v1:
1. pstore.backend no longer acts as "registered backend", but "backends
eligible for registration".
2. drop subdir since it will break user space
3. drop tty frontend since I haven't yet devised a satisfactory
implementation strategy
Changes from v2:
1. Fix ftrace.c build error as I did not compile with
CONFIG_PSTORE_FTRACE.
A heartfelt thank you to Kees and Guilherme for your suggestions.
I firmly believe that a tty frontend is crucial for kdump debugging,
and I am still dedicating effort to develop one. Hope in the future I
can accomplish it with deeper comprehension with tty driver :)
Yuanhe Shu (3):
pstore: add multi-backend support
Documentation: adjust pstore backend related document
tools/testing: adjust pstore backend related selftest
Documentation/ABI/testing/pstore | 8 +-
.../admin-guide/kernel-parameters.txt | 4 +-
fs/pstore/ftrace.c | 31 ++-
fs/pstore/inode.c | 19 +-
fs/pstore/internal.h | 4 +-
fs/pstore/platform.c | 225 ++++++++++++------
fs/pstore/pmsg.c | 24 +-
include/linux/pstore.h | 29 +++
tools/testing/selftests/pstore/common_tests | 8 +-
.../selftests/pstore/pstore_post_reboot_tests | 65 ++---
tools/testing/selftests/pstore/pstore_tests | 2 +-
11 files changed, 295 insertions(+), 124 deletions(-)
--
2.39.3
Pstore now supports multiple backends, the module parameter
pstore.backend varies from 'registered backend' to 'backends that are
allowed to register'. Adjust selftests to match the change.
Signed-off-by: Yuanhe Shu <[email protected]>
---
tools/testing/selftests/pstore/common_tests | 8 +--
.../selftests/pstore/pstore_post_reboot_tests | 65 ++++++++++---------
tools/testing/selftests/pstore/pstore_tests | 2 +-
3 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
index 4509f0cc9c91..497e6fc3215f 100755
--- a/tools/testing/selftests/pstore/common_tests
+++ b/tools/testing/selftests/pstore/common_tests
@@ -27,9 +27,9 @@ show_result() { # result_value
}
check_files_exist() { # type of pstorefs file
- if [ -e ${1}-${backend}-0 ]; then
+ if [ -e ${1}-${2}-0 ]; then
prlog "ok"
- for f in `ls ${1}-${backend}-*`; do
+ for f in `ls ${1}-${2}-*`; do
prlog -e "\t${f}"
done
else
@@ -74,9 +74,9 @@ prlog "=== Pstore unit tests (`basename $0`) ==="
prlog "UUID="$UUID
prlog -n "Checking pstore backend is registered ... "
-backend=`cat /sys/module/pstore/parameters/backend`
+backends=$(dmesg | sed -n 's/.*pstore: Registered \(.*\) as persistent store backend.*/\1/p')
show_result $?
-prlog -e "\tbackend=${backend}"
+prlog -e "\tbackends="$backends
prlog -e "\tcmdline=`cat /proc/cmdline`"
if [ $rc -ne 0 ]; then
exit 1
diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
index d6da5e86efbf..9e40ccb9c918 100755
--- a/tools/testing/selftests/pstore/pstore_post_reboot_tests
+++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
@@ -36,45 +36,46 @@ else
fi
cd ${mount_point}
+for backend in ${backends}; do
+ prlog -n "Checking ${backend}-dmesg files exist in pstore filesystem ... "
+ check_files_exist dmesg ${backend}
-prlog -n "Checking dmesg files exist in pstore filesystem ... "
-check_files_exist dmesg
+ prlog -n "Checking ${backend}-console files exist in pstore filesystem ... "
+ check_files_exist console ${backend}
-prlog -n "Checking console files exist in pstore filesystem ... "
-check_files_exist console
+ prlog -n "Checking ${backend}-pmsg files exist in pstore filesystem ... "
+ check_files_exist pmsg ${backend}
-prlog -n "Checking pmsg files exist in pstore filesystem ... "
-check_files_exist pmsg
+ prlog -n "Checking ${backend}-dmesg files contain oops end marker"
+ grep_end_trace() {
+ grep -q "\---\[ end trace" $1
+ }
+ files=`ls dmesg-${backend}-*`
+ operate_files $? "$files" grep_end_trace
-prlog -n "Checking dmesg files contain oops end marker"
-grep_end_trace() {
- grep -q "\---\[ end trace" $1
-}
-files=`ls dmesg-${backend}-*`
-operate_files $? "$files" grep_end_trace
+ prlog -n "Checking ${backend}-console file contains oops end marker ... "
+ grep -q "\---\[ end trace" console-${backend}-0
+ show_result $?
-prlog -n "Checking console file contains oops end marker ... "
-grep -q "\---\[ end trace" console-${backend}-0
-show_result $?
-
-prlog -n "Checking pmsg file properly keeps the content written before crash ... "
-prev_uuid=`cat $TOP_DIR/prev_uuid`
-if [ $? -eq 0 ]; then
- nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
- if [ $nr_matched -eq 1 ]; then
- grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
- show_result $?
+ prlog -n "Checking ${backend}-pmsg file properly keeps the content written before crash ... "
+ prev_uuid=`cat $TOP_DIR/prev_uuid`
+ if [ $? -eq 0 ]; then
+ nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
+ if [ $nr_matched -eq 1 ]; then
+ grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
+ show_result $?
+ else
+ prlog "FAIL"
+ rc=1
+ fi
else
- prlog "FAIL"
- rc=1
+ prlog "FAIL"
+ rc=1
fi
-else
- prlog "FAIL"
- rc=1
-fi
-prlog -n "Removing all files in pstore filesystem "
-files=`ls *-${backend}-*`
-operate_files $? "$files" rm
+ prlog -n "Removing all ${backend} files in pstore filesystem "
+ files=`ls *-${backend}-*`
+ operate_files $? "$files" rm
+done
exit $rc
diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
index 2aa9a3852a84..f4665a8c77dc 100755
--- a/tools/testing/selftests/pstore/pstore_tests
+++ b/tools/testing/selftests/pstore/pstore_tests
@@ -10,7 +10,7 @@
. ./common_tests
prlog -n "Checking pstore console is registered ... "
-dmesg | grep -Eq "console \[(pstore|${backend})"
+dmesg | grep -Eq "console \[(pstore console)"
show_result $?
prlog -n "Checking /dev/pmsg0 exists ... "
--
2.39.3
On Wed, Feb 07, 2024 at 10:19:21AM +0800, Yuanhe Shu wrote:
> Pstore now supports multiple backends, the module parameter
> pstore.backend varies from 'registered backend' to 'backends that are
> allowed to register'. Adjust selftests to match the change.
>
> Signed-off-by: Yuanhe Shu <[email protected]>
> ---
> tools/testing/selftests/pstore/common_tests | 8 +--
> .../selftests/pstore/pstore_post_reboot_tests | 65 ++++++++++---------
> tools/testing/selftests/pstore/pstore_tests | 2 +-
> 3 files changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
> index 4509f0cc9c91..497e6fc3215f 100755
> --- a/tools/testing/selftests/pstore/common_tests
> +++ b/tools/testing/selftests/pstore/common_tests
> @@ -27,9 +27,9 @@ show_result() { # result_value
> }
>
> check_files_exist() { # type of pstorefs file
> - if [ -e ${1}-${backend}-0 ]; then
> + if [ -e ${1}-${2}-0 ]; then
> prlog "ok"
> - for f in `ls ${1}-${backend}-*`; do
> + for f in `ls ${1}-${2}-*`; do
> prlog -e "\t${f}"
> done
> else
> @@ -74,9 +74,9 @@ prlog "=== Pstore unit tests (`basename $0`) ==="
> prlog "UUID="$UUID
>
> prlog -n "Checking pstore backend is registered ... "
> -backend=`cat /sys/module/pstore/parameters/backend`
> +backends=$(dmesg | sed -n 's/.*pstore: Registered \(.*\) as persistent store backend.*/\1/p')
> show_result $?
> -prlog -e "\tbackend=${backend}"
> +prlog -e "\tbackends="$backends
Missing trailing "? Also, doesn't this end up printing multiple lines?
Perhaps, like LSM stacking, we need a /sys/module entry for the list of
backends, comma separated?
> prlog -e "\tcmdline=`cat /proc/cmdline`"
> if [ $rc -ne 0 ]; then
> exit 1
> diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> index d6da5e86efbf..9e40ccb9c918 100755
> --- a/tools/testing/selftests/pstore/pstore_post_reboot_tests
> +++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
> @@ -36,45 +36,46 @@ else
> fi
>
> cd ${mount_point}
> +for backend in ${backends}; do
> + prlog -n "Checking ${backend}-dmesg files exist in pstore filesystem ... "
> + check_files_exist dmesg ${backend}
>
> -prlog -n "Checking dmesg files exist in pstore filesystem ... "
> -check_files_exist dmesg
> + prlog -n "Checking ${backend}-console files exist in pstore filesystem ... "
> + check_files_exist console ${backend}
>
> -prlog -n "Checking console files exist in pstore filesystem ... "
> -check_files_exist console
> + prlog -n "Checking ${backend}-pmsg files exist in pstore filesystem ... "
> + check_files_exist pmsg ${backend}
>
> -prlog -n "Checking pmsg files exist in pstore filesystem ... "
> -check_files_exist pmsg
> + prlog -n "Checking ${backend}-dmesg files contain oops end marker"
> + grep_end_trace() {
> + grep -q "\---\[ end trace" $1
> + }
> + files=`ls dmesg-${backend}-*`
> + operate_files $? "$files" grep_end_trace
>
> -prlog -n "Checking dmesg files contain oops end marker"
> -grep_end_trace() {
> - grep -q "\---\[ end trace" $1
> -}
> -files=`ls dmesg-${backend}-*`
> -operate_files $? "$files" grep_end_trace
> + prlog -n "Checking ${backend}-console file contains oops end marker ... "
> + grep -q "\---\[ end trace" console-${backend}-0
> + show_result $?
>
> -prlog -n "Checking console file contains oops end marker ... "
> -grep -q "\---\[ end trace" console-${backend}-0
> -show_result $?
> -
> -prlog -n "Checking pmsg file properly keeps the content written before crash ... "
> -prev_uuid=`cat $TOP_DIR/prev_uuid`
> -if [ $? -eq 0 ]; then
> - nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
> - if [ $nr_matched -eq 1 ]; then
> - grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
> - show_result $?
> + prlog -n "Checking ${backend}-pmsg file properly keeps the content written before crash ... "
> + prev_uuid=`cat $TOP_DIR/prev_uuid`
> + if [ $? -eq 0 ]; then
> + nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
> + if [ $nr_matched -eq 1 ]; then
> + grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
> + show_result $?
> + else
> + prlog "FAIL"
> + rc=1
> + fi
> else
> - prlog "FAIL"
> - rc=1
> + prlog "FAIL"
> + rc=1
> fi
> -else
> - prlog "FAIL"
> - rc=1
> -fi
>
> -prlog -n "Removing all files in pstore filesystem "
> -files=`ls *-${backend}-*`
> -operate_files $? "$files" rm
> + prlog -n "Removing all ${backend} files in pstore filesystem "
> + files=`ls *-${backend}-*`
> + operate_files $? "$files" rm
> +done
>
> exit $rc
> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
> index 2aa9a3852a84..f4665a8c77dc 100755
> --- a/tools/testing/selftests/pstore/pstore_tests
> +++ b/tools/testing/selftests/pstore/pstore_tests
> @@ -10,7 +10,7 @@
> . ./common_tests
>
> prlog -n "Checking pstore console is registered ... "
> -dmesg | grep -Eq "console \[(pstore|${backend})"
> +dmesg | grep -Eq "console \[(pstore console)"
> show_result $?
>
> prlog -n "Checking /dev/pmsg0 exists ... "
> --
> 2.39.3
>
Otherwise seems ok
--
Kees Cook
On 2024/2/7 20:53, Kees Cook wrote:
> On Wed, Feb 07, 2024 at 10:19:21AM +0800, Yuanhe Shu wrote:
>> Pstore now supports multiple backends, the module parameter
>> pstore.backend varies from 'registered backend' to 'backends that are
>> allowed to register'. Adjust selftests to match the change.
>>
>> Signed-off-by: Yuanhe Shu <[email protected]>
>> ---
>> tools/testing/selftests/pstore/common_tests | 8 +--
>> .../selftests/pstore/pstore_post_reboot_tests | 65 ++++++++++---------
>> tools/testing/selftests/pstore/pstore_tests | 2 +-
>> 3 files changed, 38 insertions(+), 37 deletions(-)
>>
>> diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
>> index 4509f0cc9c91..497e6fc3215f 100755
>> --- a/tools/testing/selftests/pstore/common_tests
>> +++ b/tools/testing/selftests/pstore/common_tests
>> @@ -27,9 +27,9 @@ show_result() { # result_value
>> }
>>
>> check_files_exist() { # type of pstorefs file
>> - if [ -e ${1}-${backend}-0 ]; then
>> + if [ -e ${1}-${2}-0 ]; then
>> prlog "ok"
>> - for f in `ls ${1}-${backend}-*`; do
>> + for f in `ls ${1}-${2}-*`; do
>> prlog -e "\t${f}"
>> done
>> else
>> @@ -74,9 +74,9 @@ prlog "=== Pstore unit tests (`basename $0`) ==="
>> prlog "UUID="$UUID
>>
>> prlog -n "Checking pstore backend is registered ... "
>> -backend=`cat /sys/module/pstore/parameters/backend`
>> +backends=$(dmesg | sed -n 's/.*pstore: Registered \(.*\) as persistent store backend.*/\1/p')
>> show_result $?
>> -prlog -e "\tbackend=${backend}"
>> +prlog -e "\tbackends="$backends
>
> Missing trailing "? Also, doesn't this end up printing multiple lines?
> Perhaps, like LSM stacking, we need a /sys/module entry for the list of
> backends, comma separated?
>
To avoid printing multiple lines here we move $backends out of "" then
it will print one single line backend names seperated by white space.
Yes, I also referred to LSM stacking and wondering if we need a module
parameter to indicate which backends are registered at present. It would
be nice for users to know which pstore backends are registered and
selftest could take it for test easily. But I am worried about it would
be confusing for users that there is a parameter pstore.backend to
indicate which backends are allowed to be registered and another
parameter to indicate which backends are registered now. At first the
naming is a question. What is your advice?
>> prlog -e "\tcmdline=`cat /proc/cmdline`"
>> if [ $rc -ne 0 ]; then
>> exit 1
>> diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests b/tools/testing/selftests/pstore/pstore_post_reboot_tests
>> index d6da5e86efbf..9e40ccb9c918 100755
>> --- a/tools/testing/selftests/pstore/pstore_post_reboot_tests
>> +++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
>> @@ -36,45 +36,46 @@ else
>> fi
>>
>> cd ${mount_point}
>> +for backend in ${backends}; do
>> + prlog -n "Checking ${backend}-dmesg files exist in pstore filesystem ... "
>> + check_files_exist dmesg ${backend}
>>
>> -prlog -n "Checking dmesg files exist in pstore filesystem ... "
>> -check_files_exist dmesg
>> + prlog -n "Checking ${backend}-console files exist in pstore filesystem ... "
>> + check_files_exist console ${backend}
>>
>> -prlog -n "Checking console files exist in pstore filesystem ... "
>> -check_files_exist console
>> + prlog -n "Checking ${backend}-pmsg files exist in pstore filesystem ... "
>> + check_files_exist pmsg ${backend}
>>
>> -prlog -n "Checking pmsg files exist in pstore filesystem ... "
>> -check_files_exist pmsg
>> + prlog -n "Checking ${backend}-dmesg files contain oops end marker"
>> + grep_end_trace() {
>> + grep -q "\---\[ end trace" $1
>> + }
>> + files=`ls dmesg-${backend}-*`
>> + operate_files $? "$files" grep_end_trace
>>
>> -prlog -n "Checking dmesg files contain oops end marker"
>> -grep_end_trace() {
>> - grep -q "\---\[ end trace" $1
>> -}
>> -files=`ls dmesg-${backend}-*`
>> -operate_files $? "$files" grep_end_trace
>> + prlog -n "Checking ${backend}-console file contains oops end marker ... "
>> + grep -q "\---\[ end trace" console-${backend}-0
>> + show_result $?
>>
>> -prlog -n "Checking console file contains oops end marker ... "
>> -grep -q "\---\[ end trace" console-${backend}-0
>> -show_result $?
>> -
>> -prlog -n "Checking pmsg file properly keeps the content written before crash ... "
>> -prev_uuid=`cat $TOP_DIR/prev_uuid`
>> -if [ $? -eq 0 ]; then
>> - nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
>> - if [ $nr_matched -eq 1 ]; then
>> - grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
>> - show_result $?
>> + prlog -n "Checking ${backend}-pmsg file properly keeps the content written before crash ... "
>> + prev_uuid=`cat $TOP_DIR/prev_uuid`
>> + if [ $? -eq 0 ]; then
>> + nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
>> + if [ $nr_matched -eq 1 ]; then
>> + grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
>> + show_result $?
>> + else
>> + prlog "FAIL"
>> + rc=1
>> + fi
>> else
>> - prlog "FAIL"
>> - rc=1
>> + prlog "FAIL"
>> + rc=1
>> fi
>> -else
>> - prlog "FAIL"
>> - rc=1
>> -fi
>>
>> -prlog -n "Removing all files in pstore filesystem "
>> -files=`ls *-${backend}-*`
>> -operate_files $? "$files" rm
>> + prlog -n "Removing all ${backend} files in pstore filesystem "
>> + files=`ls *-${backend}-*`
>> + operate_files $? "$files" rm
>> +done
>>
>> exit $rc
>> diff --git a/tools/testing/selftests/pstore/pstore_tests b/tools/testing/selftests/pstore/pstore_tests
>> index 2aa9a3852a84..f4665a8c77dc 100755
>> --- a/tools/testing/selftests/pstore/pstore_tests
>> +++ b/tools/testing/selftests/pstore/pstore_tests
>> @@ -10,7 +10,7 @@
>> . ./common_tests
>>
>> prlog -n "Checking pstore console is registered ... "
>> -dmesg | grep -Eq "console \[(pstore|${backend})"
>> +dmesg | grep -Eq "console \[(pstore console)"
>> show_result $?
>>
>> prlog -n "Checking /dev/pmsg0 exists ... "
>> --
>> 2.39.3
>>
>
> Otherwise seems ok
>