Fixes for procfs files backed by linked lists

There are some issues with the way the seq_file interface is implemented
for kstats backed by linked lists (zfs_dbgmsgs and certain per-pool
debugging info):

* We don't account for the fact that seq_file sometimes visits a node
  multiple times, which results in missing messages when read through
  procfs.
* We don't keep separate state for each reader of a file, so concurrent
  readers will receive incorrect results.
* We don't account for the fact that entries may have been removed from
  the list between read syscalls, so reading from these files in procfs
  can cause the system to crash.

This change fixes these issues and adds procfs_list, a wrapper around a
linked list which abstracts away the details of implementing the
seq_file interface for a list and exposing the contents of the list
through procfs.

Reviewed by: Don Brady <[email protected]>
Reviewed-by: Serapheim Dimitropoulos <[email protected]>
Reviewed by: Brad Lewis <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: John Gallagher <[email protected]>
External-issue: LX-1211
Closes #7819 

10 files changed, 294 insertions, 14 deletions

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index 4b41c3f74..95e70f043 100644
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -584,10 +584,6 @@ tests = ['inuse_001_pos', 'inuse_003_pos', 'inuse_004_pos',
 post =
 tags = ['functional', 'inuse']
 
-[tests/functional/kstat]
-tests = ['state']
-tags = ['functional', 'kstat']
-
 [tests/functional/large_files]
 tests = ['large_files_001_pos', 'large_files_002_pos']
 tags = ['functional', 'large_files']
@@ -672,6 +668,11 @@ tags = ['functional', 'poolversion']
 tests = ['privilege_001_pos', 'privilege_002_pos']
 tags = ['functional', 'privilege']
 
+[tests/functional/procfs]
+tests = ['procfs_list_basic', 'procfs_list_concurrent_readers',
+    'procfs_list_stale_read', 'pool_state']
+tags = ['functional', 'procfs']
+
 [tests/functional/projectquota]
 tests = ['projectid_001_pos', 'projectid_002_pos', 'projectid_003_pos',
     'projectquota_001_pos', 'projectquota_002_pos', 'projectquota_003_pos',
diff --git a/tests/zfs-tests/tests/functional/Makefile.am b/tests/zfs-tests/tests/functional/Makefile.am
index e0a4aca99..961a34027 100644
--- a/tests/zfs-tests/tests/functional/Makefile.am
+++ b/tests/zfs-tests/tests/functional/Makefile.am
@@ -29,7 +29,6 @@ SUBDIRS = \
 	inheritance \
 	inuse \
 	io \
-	kstat \
 	large_files \
 	largest_pool \
 	libzfs \
@@ -48,6 +47,7 @@ SUBDIRS = \
 	pool_names \
 	poolversion \
 	privilege \
+	procfs \
 	projectquota \
 	quota \
 	raidz \
diff --git a/tests/zfs-tests/tests/functional/kstat/Makefile.am b/tests/zfs-tests/tests/functional/kstat/Makefile.am
deleted file mode 100644
index 8ad83ec3e..000000000
--- a/tests/zfs-tests/tests/functional/kstat/Makefile.am
+++ /dev/null
@@ -1,5 +0,0 @@
-pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/kstat
-dist_pkgdata_SCRIPTS = \
-	setup.ksh \
-	cleanup.ksh \
-	state.ksh
diff --git a/tests/zfs-tests/tests/functional/procfs/Makefile.am b/tests/zfs-tests/tests/functional/procfs/Makefile.am
new file mode 100644
index 000000000..a7f022d9f
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/procfs/Makefile.am
@@ -0,0 +1,8 @@
+pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/procfs
+dist_pkgdata_SCRIPTS = \
+	setup.ksh \
+	cleanup.ksh \
+	procfs_list_basic.ksh \
+	procfs_list_concurrent_readers.ksh \
+	procfs_list_stale_read.ksh \
+	pool_state.ksh
diff --git a/tests/zfs-tests/tests/functional/kstat/cleanup.ksh b/tests/zfs-tests/tests/functional/procfs/cleanup.ksh
index 8a212ce37..8fe46577e 100755
--- a/tests/zfs-tests/tests/functional/kstat/cleanup.ksh
+++ b/tests/zfs-tests/tests/functional/procfs/cleanup.ksh
@@ -19,8 +19,9 @@
 #
 # CDDL HEADER END
 #
+
 #
-# Copyright (c) 2018 by Lawrence Livermore National Security, LLC.
+# Copyright (c) 2018 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
diff --git a/tests/zfs-tests/tests/functional/kstat/state.ksh b/tests/zfs-tests/tests/functional/procfs/pool_state.ksh
index 3c29266e5..3c29266e5 100755
--- a/tests/zfs-tests/tests/functional/kstat/state.ksh
+++ b/tests/zfs-tests/tests/functional/procfs/pool_state.ksh
diff --git a/tests/zfs-tests/tests/functional/procfs/procfs_list_basic.ksh b/tests/zfs-tests/tests/functional/procfs/procfs_list_basic.ksh
new file mode 100755
index 000000000..c9eff3649
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/procfs/procfs_list_basic.ksh
@@ -0,0 +1,95 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+# Test that we can read from and write to a file in procfs whose contents is
+# backed by a linked list.
+#
+# STRATEGY:
+# 1. Take some snapshots of a filesystem, which will cause some messages to be
+#    written to the zfs dbgmsgs.
+# 2. Read the dbgmsgs via procfs and verify that the expected messages are
+#    present.
+# 3. Write to the dbgmsgs file to clear the messages.
+# 4. Read the dbgmsgs again, and make sure the messages are no longer present.
+#
+
+function cleanup
+{
+	datasetexists $FS && log_must zfs destroy -r $FS
+}
+
+function count_snap_cmds
+{
+	typeset expected_count=$1
+	count=$(grep "command: zfs snapshot $FS@testsnapshot" | wc -l)
+	log_must eval "[[ $count -eq $expected_count ]]"
+}
+
+typeset -r ZFS_DBGMSG=/proc/spl/kstat/zfs/dbgmsg
+typeset -r FS=$TESTPOOL/fs
+typeset snap_msgs
+
+log_onexit cleanup
+
+# Clear out old messages
+echo 0 >$ZFS_DBGMSG || log_fail "failed to write to $ZFS_DBGMSG"
+
+log_must zfs create $FS
+for i in {1..20}; do
+	log_must zfs snapshot "$FS@testsnapshot$i"
+done
+log_must zpool sync $TESTPOOL
+
+#
+# Read the debug message file in small chunks to make sure that the read is
+# split up into multiple syscalls. This tests that when a syscall begins we
+# correctly pick up in the list of messages where the previous syscall left
+# off. The size of the read can affect how many bytes the seq_file code has
+# left in its internal buffer, which in turn can affect the relative pos that
+# the seq_file code picks up at when the next read starts. Try a few
+# different size reads to make sure we can handle each case.
+#
+# Check that the file has the right contents by grepping for some of the
+# messages that we expect to be present.
+#
+for chunk_sz in {1,64,256,1024,4096}; do
+	dd if=$ZFS_DBGMSG bs=$chunk_sz | count_snap_cmds 20
+done
+
+# Clear out old messages and check that they really are gone
+echo 0 >$ZFS_DBGMSG || log_fail "failed to write to $ZFS_DBGMSG"
+cat $ZFS_DBGMSG | count_snap_cmds 0
+#
+# Even though we don't expect any messages in the file, reading should still
+# succeed.
+#
+log_must cat $ZFS_DBGMSG
+
+log_pass "Basic reading/writing of procfs file backed by linked list successful"
diff --git a/tests/zfs-tests/tests/functional/procfs/procfs_list_concurrent_readers.ksh b/tests/zfs-tests/tests/functional/procfs/procfs_list_concurrent_readers.ksh
new file mode 100755
index 000000000..473de5c84
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/procfs/procfs_list_concurrent_readers.ksh
@@ -0,0 +1,82 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+# Make sure that interleaving reads from different readers does not affect the
+# results that are returned.
+#
+# STRATEGY:
+# 1. Make sure a few debug messages have been logged.
+# 2. Open the procfs file and start reading from it.
+# 3. Open the file again, and read its entire contents.
+# 4. Resume reading from the first instance.
+# 5. Check that the contents read by the two instances are identical.
+#
+
+function cleanup
+{
+	[[ -z $msgs1 ]] || log_must rm $msgs1
+	[[ -z $msgs2 ]] || log_must rm $msgs2
+	datasetexists $FS && log_must zfs destroy -r $FS
+}
+
+typeset -r ZFS_DBGMSG=/proc/spl/kstat/zfs/dbgmsg
+typeset -r FS=$TESTPOOL/fs
+typeset msgs1 msgs2
+
+log_onexit cleanup
+
+# Clear out old messages
+echo 0 >$ZFS_DBGMSG || log_fail "failed to write to $ZFS_DBGMSG"
+
+# Add some new messages
+log_must zfs create $FS
+for i in {1..20}; do
+	log_must zfs snapshot "$FS@testsnapshot$i"
+done
+log_must zpool sync $TESTPOOL
+
+msgs1=$(mktemp) || log_fail
+msgs2=$(mktemp) || log_fail
+
+#
+# Start reading file, pause and read it from another process, and then finish
+# reading.
+#
+{ dd bs=512 count=4; cat $ZFS_DBGMSG >$msgs1; cat; } <$ZFS_DBGMSG >$msgs2
+
+#
+# Truncate the result of the read that completed second in case it picked up an
+# extra message that was logged after the first read completed.
+#
+log_must truncate -s $(stat -c "%s" $msgs1) $msgs2
+
+log_must diff $msgs1 $msgs2
+
+log_pass "Concurrent readers receive identical results"
diff --git a/tests/zfs-tests/tests/functional/procfs/procfs_list_stale_read.ksh b/tests/zfs-tests/tests/functional/procfs/procfs_list_stale_read.ksh
new file mode 100755
index 000000000..c363e7f8b
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/procfs/procfs_list_stale_read.ksh
@@ -0,0 +1,98 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+# Make sure errors caused by messages being dropped from the list backing the
+# procfs file are handled gracefully.
+#
+# STRATEGY:
+# 1. Make sure a few entries have been logged.
+# 2. Open the procfs file and start reading from it.
+# 3. Write to the file to cause its contents to be dropped.
+# 4. Resume reading from the first instance, and check that the expected
+#    error is received.
+# 5. Repeat steps 1-4, except instead of dropping all the messages by writing
+#    to the file, cause enough new messages to be written that the old messages
+#    are dropped.
+#
+
+function cleanup
+{
+	echo $default_max_entries >$MAX_ENTRIES_PARAM || log_fail
+}
+
+function sync_n
+{
+	for i in {1..$1}; do
+		log_must zpool sync $TESTPOOL
+	done
+	return 0
+}
+
+function do_test
+{
+	typeset cmd=$1
+
+	# Clear out old entries
+	echo 0 >$TXG_HIST || log_fail
+
+	# Add some new entries
+	sync_n 20
+
+	# Confirm that there actually is something in the file.
+	[[ $(wc -l <$TXG_HIST) -ge 20 ]] || log_fail "expected more entries"
+
+	#
+	# Start reading file, pause and run a command that will cause the
+	# current offset into the file to become invalid, and then try to
+	# finish reading.
+	#
+	{
+		log_must dd bs=512 count=4 >/dev/null
+		log_must eval "$cmd"
+		cat 2>&1 >/dev/null | log_must grep "Input/output error"
+	} <$TXG_HIST
+}
+
+typeset -r TXG_HIST=/proc/spl/kstat/zfs/$TESTPOOL/txgs
+typeset MAX_ENTRIES_PARAM=/sys/module/zfs/parameters/zfs_txg_history
+typeset default_max_entries
+
+log_onexit cleanup
+
+default_max_entries=$(cat $MAX_ENTRIES_PARAM) || log_fail
+echo 50 >$MAX_ENTRIES_PARAM || log_fail
+
+# Clear all of the existing entries.
+do_test "echo 0 >$TXG_HIST"
+
+# Add enough new entries to the list that all of the old ones are dropped.
+do_test "sync_n 60"
+
+log_pass "Attempting to read dropped message returns expected error"
diff --git a/tests/zfs-tests/tests/functional/kstat/setup.ksh b/tests/zfs-tests/tests/functional/procfs/setup.ksh
index 57717a096..b3812dbdc 100755
--- a/tests/zfs-tests/tests/functional/kstat/setup.ksh
+++ b/tests/zfs-tests/tests/functional/procfs/setup.ksh
@@ -19,16 +19,16 @@
 #
 # CDDL HEADER END
 #
+
 #
-# Copyright (c) 2018 by Lawrence Livermore National Security, LLC.
+# Copyright (c) 2018 by Delphix. All rights reserved.
 #
 
 . $STF_SUITE/include/libtest.shlib
 
 if ! is_linux ; then
-	log_unsupported "/proc/spl/kstat/<pool>/health only supported on Linux"
+	log_unsupported "procfs is only used on Linux"
 fi
 
 default_mirror_setup $DISKS
-
 log_pass