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 openzfs#7819

Author: John Gallagher

configure.ac

@@ -283,7 +283,6 @@ AC_CONFIG_FILES([
 	tests/zfs-tests/tests/functional/inheritance/Makefile
 	tests/zfs-tests/tests/functional/inuse/Makefile
 	tests/zfs-tests/tests/functional/io/Makefile
-	tests/zfs-tests/tests/functional/kstat/Makefile
 	tests/zfs-tests/tests/functional/large_files/Makefile
 	tests/zfs-tests/tests/functional/largest_pool/Makefile
 	tests/zfs-tests/tests/functional/link_count/Makefile
@@ -301,6 +300,7 @@ AC_CONFIG_FILES([
 	tests/zfs-tests/tests/functional/pool_checkpoint/Makefile
 	tests/zfs-tests/tests/functional/poolversion/Makefile
 	tests/zfs-tests/tests/functional/privilege/Makefile
+	tests/zfs-tests/tests/functional/procfs/Makefile
 	tests/zfs-tests/tests/functional/projectquota/Makefile
 	tests/zfs-tests/tests/functional/pyzfs/Makefile
 	tests/zfs-tests/tests/functional/quota/Makefile

include/spl/sys/Makefile.am

@@ -28,6 +28,7 @@ KERNEL_H = \
 	$(top_srcdir)/include/spl/sys/param.h \
 	$(top_srcdir)/include/spl/sys/processor.h \
 	$(top_srcdir)/include/spl/sys/proc.h \
+	$(top_srcdir)/include/spl/sys/procfs_list.h \
 	$(top_srcdir)/include/spl/sys/random.h \
 	$(top_srcdir)/include/spl/sys/rwlock.h \
 	$(top_srcdir)/include/spl/sys/shrinker.h \

include/spl/sys/kstat.h

@@ -98,30 +98,34 @@ typedef struct kstat_raw_ops {
 	void *(*addr)(kstat_t *ksp, loff_t index);
 } kstat_raw_ops_t;
 
+typedef struct kstat_proc_entry {
+	char	kpe_name[KSTAT_STRLEN+1];	/* kstat name */
+	char	kpe_module[KSTAT_STRLEN+1];	/* provider module name */
+	kstat_module_t		*kpe_owner;	/* kstat module linkage */
+	struct list_head	kpe_list;	/* kstat linkage */
+	struct proc_dir_entry	*kpe_proc;	/* procfs entry */
+} kstat_proc_entry_t;
+
 struct kstat_s {
 	int		ks_magic;		/* magic value */
 	kid_t		ks_kid;			/* unique kstat ID */
 	hrtime_t	ks_crtime;		/* creation time */
 	hrtime_t	ks_snaptime;		/* last access time */
-	char		ks_module[KSTAT_STRLEN+1]; /* provider module name */
 	int		ks_instance;		/* provider module instance */
-	char		ks_name[KSTAT_STRLEN+1]; /* kstat name */
 	char		ks_class[KSTAT_STRLEN+1]; /* kstat class */
 	uchar_t		ks_type;		/* kstat data type */
 	uchar_t		ks_flags;		/* kstat flags */
 	void		*ks_data;		/* kstat type-specific data */
 	uint_t		ks_ndata;		/* # of data records */
 	size_t		ks_data_size;		/* size of kstat data section */
-	struct proc_dir_entry *ks_proc;		/* proc linkage */
 	kstat_update_t	*ks_update;		/* dynamic updates */
 	void		*ks_private;		/* private data */
 	kmutex_t	ks_private_lock;	/* kstat private data lock */
 	kmutex_t	*ks_lock;		/* kstat data lock */
-	struct list_head ks_list;		/* kstat linkage */
-	kstat_module_t	*ks_owner;		/* kstat module linkage */
 	kstat_raw_ops_t	ks_raw_ops;		/* ops table for raw type */
 	char		*ks_raw_buf;		/* buf used for raw ops */
 	size_t		ks_raw_bufsize;		/* size of raw ops buffer */
+	kstat_proc_entry_t	ks_proc;	/* data for procfs entry */
 };
 
 typedef struct kstat_named_s {
@@ -189,6 +193,12 @@ extern kstat_t *__kstat_create(const char *ks_module, int ks_instance,
     const char *ks_name, const char *ks_class, uchar_t ks_type,
     uint_t ks_ndata, uchar_t ks_flags);
 
+extern void kstat_proc_entry_init(kstat_proc_entry_t *kpep,
+    const char *module, const char *name);
+extern void kstat_proc_entry_delete(kstat_proc_entry_t *kpep);
+extern void kstat_proc_entry_install(kstat_proc_entry_t *kpep,
+    const struct file_operations *file_ops, void *data);
+
 extern void __kstat_install(kstat_t *ksp);
 extern void __kstat_delete(kstat_t *ksp);
 extern void kstat_waitq_enter(kstat_io_t *);

include/spl/sys/procfs_list.h

@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+
+#ifndef	_SPL_PROCFS_LIST_H
+#define	_SPL_PROCFS_LIST_H
+
+#include <sys/kstat.h>
+#include <sys/mutex.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+
+typedef struct procfs_list procfs_list_t;
+struct procfs_list {
+	/* Accessed only by user of a procfs_list */
+	void		*pl_private;
+
+	/*
+	 * Accessed both by user of a procfs_list and by procfs_list
+	 * implementation
+	 */
+	kmutex_t	pl_lock;
+	list_t		pl_list;
+
+	/* Accessed only by procfs_list implementation */
+	uint64_t	pl_next_id;
+	int		(*pl_show)(struct seq_file *f, void *p);
+	int		(*pl_show_header)(struct seq_file *f);
+	int		(*pl_clear)(procfs_list_t *procfs_list);
+	size_t		pl_node_offset;
+	kstat_proc_entry_t	pl_kstat_entry;
+};
+
+typedef struct procfs_list_node {
+	list_node_t	pln_link;
+	uint64_t	pln_id;
+} procfs_list_node_t;
+
+void procfs_list_install(const char *module,
+    const char *name,
+    procfs_list_t *procfs_list,
+    int (*show)(struct seq_file *f, void *p),
+    int (*show_header)(struct seq_file *f),
+    int (*clear)(procfs_list_t *procfs_list),
+    size_t procfs_list_node_off);
+void procfs_list_uninstall(procfs_list_t *procfs_list);
+void procfs_list_destroy(procfs_list_t *procfs_list);
+
+void procfs_list_add(procfs_list_t *procfs_list, void *p);
+
+#endif	/* _SPL_PROCFS_LIST_H */

include/sys/spa.h

@@ -863,22 +863,27 @@ extern boolean_t spa_refcount_zero(spa_t *spa);
 #define	SCL_STATE_ALL	(SCL_STATE | SCL_L2ARC | SCL_ZIO)
 
 /* Historical pool statistics */
-typedef struct spa_stats_history {
+typedef struct spa_history_kstat {
 	kmutex_t		lock;
 	uint64_t		count;
 	uint64_t		size;
 	kstat_t			*kstat;
 	void			*private;
 	list_t			list;
-} spa_stats_history_t;
+} spa_history_kstat_t;
+
+typedef struct spa_history_list {
+	uint64_t		size;
+	procfs_list_t		procfs_list;
+} spa_history_list_t;
 
 typedef struct spa_stats {
-	spa_stats_history_t	read_history;
-	spa_stats_history_t	txg_history;
-	spa_stats_history_t	tx_assign_histogram;
-	spa_stats_history_t	io_history;
-	spa_stats_history_t	mmp_history;
-	spa_stats_history_t	state;		/* pool state */
+	spa_history_list_t	read_history;
+	spa_history_list_t	txg_history;
+	spa_history_kstat_t	tx_assign_histogram;
+	spa_history_kstat_t	io_history;
+	spa_history_list_t	mmp_history;
+	spa_history_kstat_t	state;		/* pool state */
 } spa_stats_t;
 
 typedef enum txg_state {
@@ -911,7 +916,7 @@ extern void spa_tx_assign_add_nsecs(spa_t *spa, uint64_t nsecs);
 extern int spa_mmp_history_set_skip(spa_t *spa, uint64_t mmp_kstat_id);
 extern int spa_mmp_history_set(spa_t *spa, uint64_t mmp_kstat_id, int io_error,
     hrtime_t duration);
-extern void *spa_mmp_history_add(spa_t *spa, uint64_t txg, uint64_t timestamp,
+extern void spa_mmp_history_add(spa_t *spa, uint64_t txg, uint64_t timestamp,
     uint64_t mmp_delay, vdev_t *vd, int label, uint64_t mmp_kstat_id,
     int error);
 

include/sys/zfs_context.h

@@ -62,6 +62,7 @@
 #include <sys/ctype.h>
 #include <sys/disp.h>
 #include <sys/trace.h>
+#include <sys/procfs_list.h>
 #include <linux/dcache_compat.h>
 #include <linux/utsname_compat.h>
 
@@ -351,6 +352,37 @@ extern void kstat_set_raw_ops(kstat_t *ksp,
     int (*data)(char *buf, size_t size, void *data),
     void *(*addr)(kstat_t *ksp, loff_t index));
 
+/*
+ * procfs list manipulation
+ */
+
+struct seq_file { };
+void seq_printf(struct seq_file *m, const char *fmt, ...);
+
+typedef struct procfs_list {
+	void		*pl_private;
+	kmutex_t	pl_lock;
+	list_t		pl_list;
+	uint64_t	pl_next_id;
+	size_t		pl_node_offset;
+} procfs_list_t;
+
+typedef struct procfs_list_node {
+	list_node_t	pln_link;
+	uint64_t	pln_id;
+} procfs_list_node_t;
+
+void procfs_list_install(const char *module,
+    const char *name,
+    procfs_list_t *procfs_list,
+    int (*show)(struct seq_file *f, void *p),
+    int (*show_header)(struct seq_file *f),
+    int (*clear)(procfs_list_t *procfs_list),
+    size_t procfs_list_node_off);
+void procfs_list_uninstall(procfs_list_t *procfs_list);
+void procfs_list_destroy(procfs_list_t *procfs_list);
+void procfs_list_add(procfs_list_t *procfs_list, void *p);
+
 /*
  * Kernel memory
  */

include/sys/zfs_debug.h

@@ -76,13 +76,6 @@ extern void __dprintf(const char *file, const char *func,
 
 extern void zfs_panic_recover(const char *fmt, ...);
 
-typedef struct zfs_dbgmsg {
-	list_node_t zdm_node;
-	time_t zdm_timestamp;
-	int zdm_size;
-	char zdm_msg[1]; /* variable length allocation */
-} zfs_dbgmsg_t;
-
 extern void zfs_dbgmsg_init(void);
 extern void zfs_dbgmsg_fini(void);
 

lib/libzpool/kernel.c

@@ -424,6 +424,57 @@ cv_broadcast(kcondvar_t *cv)
 	VERIFY0(pthread_cond_broadcast(cv));
 }
 
+/*
+ * =========================================================================
+ * procfs list
+ * =========================================================================
+ */
+
+void
+seq_printf(struct seq_file *m, const char *fmt, ...)
+{}
+
+void
+procfs_list_install(const char *module,
+    const char *name,
+    procfs_list_t *procfs_list,
+    int (*show)(struct seq_file *f, void *p),
+    int (*show_header)(struct seq_file *f),
+    int (*clear)(procfs_list_t *procfs_list),
+    size_t procfs_list_node_off)
+{
+	mutex_init(&procfs_list->pl_lock, NULL, MUTEX_DEFAULT, NULL);
+	list_create(&procfs_list->pl_list,
+	    procfs_list_node_off + sizeof (procfs_list_node_t),
+	    procfs_list_node_off + offsetof(procfs_list_node_t, pln_link));
+	procfs_list->pl_next_id = 1;
+	procfs_list->pl_node_offset = procfs_list_node_off;
+}
+
+void
+procfs_list_uninstall(procfs_list_t *procfs_list)
+{}
+
+void
+procfs_list_destroy(procfs_list_t *procfs_list)
+{
+	ASSERT(list_is_empty(&procfs_list->pl_list));
+	list_destroy(&procfs_list->pl_list);
+	mutex_destroy(&procfs_list->pl_lock);
+}
+
+#define	NODE_ID(procfs_list, obj) \
+		(((procfs_list_node_t *)(((char *)obj) + \
+		(procfs_list)->pl_node_offset))->pln_id)
+
+void
+procfs_list_add(procfs_list_t *procfs_list, void *p)
+{
+	ASSERT(MUTEX_HELD(&procfs_list->pl_lock));
+	NODE_ID(procfs_list, p) = procfs_list->pl_next_id++;
+	list_insert_tail(&procfs_list->pl_list, p);
+}
+
 /*
  * =========================================================================
  * vnode operations

module/spl/Makefile.in

@@ -18,6 +18,7 @@ $(MODULE)-objs += spl-kobj.o
 $(MODULE)-objs += spl-kstat.o
 $(MODULE)-objs += spl-mutex.o
 $(MODULE)-objs += spl-proc.o
+$(MODULE)-objs += spl-procfs-list.o
 $(MODULE)-objs += spl-rwlock.o
 $(MODULE)-objs += spl-taskq.o
 $(MODULE)-objs += spl-thread.o