2014-04-29 14:31:11

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 0/3] kdb: Infrastructure to display sequence files

This patchset started out as a simple patch to introduce the irqs
command from Android's FIQ debugger to kdb. However it has since grown
more powerful because allowing kdb to reuse existing kernel
infrastructure gives us extra opportunities.

Based on the comments at the top of irqdesc.h (plotting to take the
irq_desc structure private to kernel/irq) and the relative similarity
between FIQ debugger's irqs command and the contents /proc/interrupts
we start by adding a kdb feature to print seq_files. This forms the
foundation for a new command, interrupts.

I have also been able to implement a much more generic command,
seq_file, that can display a good number of files from pseudo
filesystems. This command is very powerful although that power does mean
care must be taken to deploy it safely. It is deliberately and by
default aimed at your foot!

Note that the risk associated with the seq_file command is why I
implemented the interrupts command in C (in principle it could have been
a kdb macro). Doing it in C codifies the need for show_interrupts() to
continue using spin locks as its locking strategy.

To give an idea of what can be done with this command. The following
seq_operations structures worked correctly and report no errors:

cpuinfo_op
extfrag_op
fragmentation_op
gpiolib_seq_ops
int_seq_ops (a.k.a. /proc/interrupts)
pagetypeinfo_op
unusable_op
vmalloc_op
zoneinfo_op

The following display the information correctly but triggered errors
(sleeping function called from invalid context) with lock debugging
enabled:

consoles_op
crypto_seq_ops
diskstats_op
partitions_op
slabinfo_op
vmstat_op

All tests are run on an ARM multi_v7_defconfig kernel (plus lots of
debug features) and halted using magic SysRq so that kdb has interrupt
context. Note also that some of the seq_operations structures hook into
driver supplied code that will only be called if that driver is enabled
so the test above are useful but cannot be exhaustive.

Daniel Thompson (3):
kdb: Add framework to display sequence files
proc: Provide access to /proc/interrupts from kdb
kdb: Implement seq_file command

fs/proc/interrupts.c | 10 +++++++++
include/linux/kdb.h | 3 +++
kernel/debug/kdb/kdb_io.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
kernel/debug/kdb/kdb_main.c | 28 +++++++++++++++++++++++++
4 files changed, 92 insertions(+)

--
1.9.0


2014-04-29 14:31:58

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 3/3] kdb: Implement seq_file command

Combining the kdb seq_file infrastructure with its symbolic lookups allows
a good sub-set of files held in pseudo filesystems to be displayed by
kdb. The seq_file command does exactly this and allows a significant
subset of pseudo files to be safely examined whilst debugging (and in
the hands of a brave expert an even bigger subset can be unsafely
examined).

Good arguments to try with this command include: cpuinfo_op,
gpiolib_seq_ops and vmalloc_op.

Signed-off-by: Daniel Thompson <[email protected]>
---
kernel/debug/kdb/kdb_main.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 0b097c8..d87731c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1734,6 +1734,32 @@ static int kdb_mm(int argc, const char **argv)
}

/*
+ * kdb_seq_file - This function implements the 'seq_file' command.
+ * seq_file address-expression
+ */
+static int kdb_seq_file(int argc, const char **argv)
+{
+ int diag;
+ unsigned long addr;
+ int nextarg;
+ long offset;
+ char *name;
+ const struct seq_operations *ops;
+
+ nextarg = 1;
+ diag = kdbgetaddrarg(argc, argv, &nextarg, &addr, &offset, &name);
+ if (diag)
+ return diag;
+
+ if (nextarg != argc+1)
+ return KDB_ARGCOUNT;
+
+ ops = (const struct seq_operations *) (addr + offset);
+ kdb_printf("Using sequence_ops at 0x%p (%s)\n", ops, name);
+ return kdb_print_seq_file(ops);
+}
+
+/*
* kdb_go - This function implements the 'go' command.
* go [address-expression]
*/
@@ -2838,6 +2864,8 @@ static void __init kdb_inittab(void)
"Display per_cpu variables", 3, KDB_REPEAT_NONE);
kdb_register_repeat("grephelp", kdb_grep_help, "",
"Display help on | grep", 0, KDB_REPEAT_NONE);
+ kdb_register_repeat("seq_file", kdb_seq_file, "",
+ "Show a seq_file using struct seq_operations", 3, KDB_REPEAT_NONE);
}

/* Execute any commands defined in kdb_cmds. */
--
1.9.0

2014-04-29 14:32:33

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 2/3] proc: Provide access to /proc/interrupts from kdb

The contents of /proc/interrupts is useful to diagnose problems during
boot up or when the system becomes unresponsive (or at least it can be if
failure is causes by interrupt problems). This command is also seen in
out-of-tree debug systems such as Android's FIQ debugger.

This change allows the file to be displayed from kdb.

Signed-off-by: Daniel Thompson <[email protected]>
---
fs/proc/interrupts.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/fs/proc/interrupts.c b/fs/proc/interrupts.c
index a352d57..1f8eeaf 100644
--- a/fs/proc/interrupts.c
+++ b/fs/proc/interrupts.c
@@ -4,6 +4,7 @@
#include <linux/irqnr.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/kdb.h>

/*
* /proc/interrupts
@@ -45,9 +46,18 @@ static const struct file_operations proc_interrupts_operations = {
.release = seq_release,
};

+#ifdef CONFIG_KGDB_KDB
+static int kdb_interrupts(int argc, const char **argv)
+{
+ return kdb_print_seq_file(&int_seq_ops);
+}
+#endif
+
static int __init proc_interrupts_init(void)
{
proc_create("interrupts", 0, NULL, &proc_interrupts_operations);
+ kdb_register("interrupts", kdb_interrupts, "",
+ "Show /proc/interrupts", 3);
return 0;
}
fs_initcall(proc_interrupts_init);
--
1.9.0

2014-04-29 14:33:40

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 1/3] kdb: Add framework to display sequence files

Lots of useful information about the system is held in pseudo filesystems
and presented using the seq_file mechanism. Unfortunately during both boot
up and kernel panic (both good times to break out kdb) it is difficult to
examine these files. This patch introduces a means to display sequence
files via kdb.

Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 3 +++
kernel/debug/kdb/kdb_io.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)

diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 290db12..2607893 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -25,6 +25,7 @@ typedef int (*kdb_func_t)(int, const char **);
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/atomic.h>
+#include <linux/seq_file.h>

#define KDB_POLL_FUNC_MAX 5
extern int kdb_poll_idx;
@@ -117,6 +118,8 @@ extern __printf(1, 0) int vkdb_printf(const char *fmt, va_list args);
extern __printf(1, 2) int kdb_printf(const char *, ...);
typedef __printf(1, 2) int (*kdb_printf_t)(const char *, ...);

+extern int kdb_print_seq_file(const struct seq_operations *ops);
+
extern void kdb_init(int level);

/* Access to kdb specific polling devices */
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 14ff484..c68c223 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -850,3 +850,54 @@ int kdb_printf(const char *fmt, ...)
return r;
}
EXPORT_SYMBOL_GPL(kdb_printf);
+
+/*
+ * Display a seq_file on the kdb console.
+ */
+
+static int __kdb_print_seq_file(struct seq_file *m, void *v)
+{
+ int i, res;
+
+ res = m->op->show(m, v);
+ if (0 != res)
+ return KDB_BADLENGTH;
+
+ for (i = 0; i < m->count && !KDB_FLAG(CMD_INTERRUPT); i++)
+ kdb_printf("%c", m->buf[i]);
+ m->count = 0;
+
+ return 0;
+}
+
+int kdb_print_seq_file(const struct seq_operations *ops)
+{
+ static char seq_buf[4096];
+ static DEFINE_SPINLOCK(seq_buf_lock);
+ unsigned long flags;
+ struct seq_file m = {
+ .buf = seq_buf,
+ .size = sizeof(seq_buf),
+ /* .lock is deliberately uninitialized to help reveal
+ * unsupportable show methods
+ */
+ .op = ops,
+ };
+ loff_t pos = 0;
+ void *v;
+ int res = 0;
+
+ v = ops->start(&m, &pos);
+ while (v) {
+ spin_lock_irqsave(&seq_buf_lock, flags);
+ res = __kdb_print_seq_file(&m, v);
+ spin_unlock_irqrestore(&seq_buf_lock, flags);
+ if (res != 0 || KDB_FLAG(CMD_INTERRUPT))
+ break;
+
+ v = ops->next(&m, v, &pos);
+ }
+ ops->stop(&m, v);
+
+ return res;
+}
--
1.9.0

2014-04-30 15:41:09

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 1/3] kdb: Add framework to display sequence files

Lots of useful information about the system is held in pseudo filesystems
and presented using the seq_file mechanism. Unfortunately during both boot
up and kernel panic (both good times to break out kdb) it is difficult to
examine these files. This patch introduces a means to display sequence
files via kdb.

Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 5 +++++
kernel/debug/kdb/kdb_io.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)

diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 290db12..fb46dd4 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -25,6 +25,7 @@ typedef int (*kdb_func_t)(int, const char **);
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/atomic.h>
+#include <linux/seq_file.h>

#define KDB_POLL_FUNC_MAX 5
extern int kdb_poll_idx;
@@ -117,6 +118,8 @@ extern __printf(1, 0) int vkdb_printf(const char *fmt, va_list args);
extern __printf(1, 2) int kdb_printf(const char *, ...);
typedef __printf(1, 2) int (*kdb_printf_t)(const char *, ...);

+extern int kdb_print_seq_file(const struct seq_operations *ops);
+
extern void kdb_init(int level);

/* Access to kdb specific polling devices */
@@ -151,6 +154,8 @@ extern int kdb_register_repeat(char *, kdb_func_t, char *, char *,
extern int kdb_unregister(char *);
#else /* ! CONFIG_KGDB_KDB */
static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; }
+static inline int
+kdb_print_seq_file(const struct seq_operations *ops) { return 0; }
static inline void kdb_init(int level) {}
static inline int kdb_register(char *cmd, kdb_func_t func, char *usage,
char *help, short minlen) { return 0; }
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 14ff484..c68c223 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -850,3 +850,54 @@ int kdb_printf(const char *fmt, ...)
return r;
}
EXPORT_SYMBOL_GPL(kdb_printf);
+
+/*
+ * Display a seq_file on the kdb console.
+ */
+
+static int __kdb_print_seq_file(struct seq_file *m, void *v)
+{
+ int i, res;
+
+ res = m->op->show(m, v);
+ if (0 != res)
+ return KDB_BADLENGTH;
+
+ for (i = 0; i < m->count && !KDB_FLAG(CMD_INTERRUPT); i++)
+ kdb_printf("%c", m->buf[i]);
+ m->count = 0;
+
+ return 0;
+}
+
+int kdb_print_seq_file(const struct seq_operations *ops)
+{
+ static char seq_buf[4096];
+ static DEFINE_SPINLOCK(seq_buf_lock);
+ unsigned long flags;
+ struct seq_file m = {
+ .buf = seq_buf,
+ .size = sizeof(seq_buf),
+ /* .lock is deliberately uninitialized to help reveal
+ * unsupportable show methods
+ */
+ .op = ops,
+ };
+ loff_t pos = 0;
+ void *v;
+ int res = 0;
+
+ v = ops->start(&m, &pos);
+ while (v) {
+ spin_lock_irqsave(&seq_buf_lock, flags);
+ res = __kdb_print_seq_file(&m, v);
+ spin_unlock_irqrestore(&seq_buf_lock, flags);
+ if (res != 0 || KDB_FLAG(CMD_INTERRUPT))
+ break;
+
+ v = ops->next(&m, v, &pos);
+ }
+ ops->stop(&m, v);
+
+ return res;
+}
--
1.9.0

2014-04-30 15:41:17

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 3/3] kdb: Implement seq_file command

Combining the kdb seq_file infrastructure with its symbolic lookups allows
a good sub-set of files held in pseudo filesystems to be displayed by
kdb. The seq_file command does exactly this and allows a significant
subset of pseudo files to be safely examined whilst debugging (and in
the hands of a brave expert an even bigger subset can be unsafely
examined).

Good arguments to try with this command include: cpuinfo_op,
gpiolib_seq_ops and vmalloc_op.

Signed-off-by: Daniel Thompson <[email protected]>
---
kernel/debug/kdb/kdb_io.c | 6 ++++--
kernel/debug/kdb/kdb_main.c | 28 ++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index c68c223..5863180 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -878,8 +878,10 @@ int kdb_print_seq_file(const struct seq_operations *ops)
struct seq_file m = {
.buf = seq_buf,
.size = sizeof(seq_buf),
- /* .lock is deliberately uninitialized to help reveal
- * unsupportable show methods
+ /* .lock is deliberately left uninitialized because it is
+ * used by seq_file read/lseek wrapper functions. It cannot be
+ * used from any of the seq_operations (assuming the ops are
+ * deadlock free with the normal interface).
*/
.op = ops,
};
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 0b097c8..d87731c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1734,6 +1734,32 @@ static int kdb_mm(int argc, const char **argv)
}

/*
+ * kdb_seq_file - This function implements the 'seq_file' command.
+ * seq_file address-expression
+ */
+static int kdb_seq_file(int argc, const char **argv)
+{
+ int diag;
+ unsigned long addr;
+ int nextarg;
+ long offset;
+ char *name;
+ const struct seq_operations *ops;
+
+ nextarg = 1;
+ diag = kdbgetaddrarg(argc, argv, &nextarg, &addr, &offset, &name);
+ if (diag)
+ return diag;
+
+ if (nextarg != argc+1)
+ return KDB_ARGCOUNT;
+
+ ops = (const struct seq_operations *) (addr + offset);
+ kdb_printf("Using sequence_ops at 0x%p (%s)\n", ops, name);
+ return kdb_print_seq_file(ops);
+}
+
+/*
* kdb_go - This function implements the 'go' command.
* go [address-expression]
*/
@@ -2838,6 +2864,8 @@ static void __init kdb_inittab(void)
"Display per_cpu variables", 3, KDB_REPEAT_NONE);
kdb_register_repeat("grephelp", kdb_grep_help, "",
"Display help on | grep", 0, KDB_REPEAT_NONE);
+ kdb_register_repeat("seq_file", kdb_seq_file, "",
+ "Show a seq_file using struct seq_operations", 3, KDB_REPEAT_NONE);
}

/* Execute any commands defined in kdb_cmds. */
--
1.9.0

2014-04-30 15:41:07

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 0/3] kdb: Infrastructure to display sequence files

This patchset started out as a simple patch to introduce the irqs
command from Android's FIQ debugger to kdb. However it has since grown
more powerful because allowing kdb to reuse existing kernel
infrastructure gives us extra opportunities.

Based on the comments at the top of irqdesc.h (plotting to take the
irq_desc structure private to kernel/irq) and the relative similarity
between FIQ debugger's irqs command and the contents /proc/interrupts
we start by adding a kdb feature to print seq_files. This forms the
foundation for a new command, interrupts.

I have also been able to implement a much more generic command,
seq_file, that can display a good number of files from pseudo
filesystems. This command is very powerful although that power does mean
care must be taken to deploy it safely. It is deliberately and by
default aimed at your foot!

Note that the risk associated with the seq_file command is why I
implemented the interrupts command in C (in principle it could have been
a kdb macro). Doing it in C codifies the need for show_interrupts() to
continue using spin locks as its locking strategy.

To give an idea of what can be done with this command. The following
seq_operations structures worked correctly and report no errors:

cpuinfo_op
extfrag_op
fragmentation_op
gpiolib_seq_ops
int_seq_ops (a.k.a. /proc/interrupts)
pagetypeinfo_op
unusable_op
vmalloc_op
zoneinfo_op

The following display the information correctly but triggered errors
(sleeping function called from invalid context) with lock debugging
enabled:

consoles_op
crypto_seq_ops
diskstats_op
partitions_op
slabinfo_op
vmstat_op

All tests are run on an ARM multi_v7_defconfig kernel (plus lots of
debug features) and halted using magic SysRq so that kdb has interrupt
context. Note also that some of the seq_operations structures hook into
driver supplied code that will only be called if that driver is enabled
so the test above are useful but cannot be exhaustive.

Changes since v1:
* Added nop inline version of kdb_print_seq_file() and used it to fix
build error when CONFIG_KGDB_KDB is not set (oops).
* Better comment explaining why seq_file's lock member can be left
uninitialized (its will not detect errors because we know it is
already not allowed for seq_file ops to use it).

Daniel Thompson (3):
kdb: Add framework to display sequence files
proc: Provide access to /proc/interrupts from kdb
kdb: Implement seq_file command

fs/proc/interrupts.c | 8 +++++++
include/linux/kdb.h | 5 +++++
kernel/debug/kdb/kdb_io.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
kernel/debug/kdb/kdb_main.c | 28 ++++++++++++++++++++++++
4 files changed, 94 insertions(+)

--
1.9.0

2014-04-30 15:41:38

by Daniel Thompson

[permalink] [raw]
Subject: [PATCH 2/3] proc: Provide access to /proc/interrupts from kdb

The contents of /proc/interrupts is useful to diagnose problems during
boot up or when the system becomes unresponsive (or at least it can be if
failure is causes by interrupt problems). This command is also seen in
out-of-tree debug systems such as Android's FIQ debugger.

This change allows the file to be displayed from kdb.

Signed-off-by: Daniel Thompson <[email protected]>
---
fs/proc/interrupts.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/fs/proc/interrupts.c b/fs/proc/interrupts.c
index a352d57..d8b64f0 100644
--- a/fs/proc/interrupts.c
+++ b/fs/proc/interrupts.c
@@ -4,6 +4,7 @@
#include <linux/irqnr.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/kdb.h>

/*
* /proc/interrupts
@@ -45,9 +46,16 @@ static const struct file_operations proc_interrupts_operations = {
.release = seq_release,
};

+static int kdb_interrupts(int argc, const char **argv)
+{
+ return kdb_print_seq_file(&int_seq_ops);
+}
+
static int __init proc_interrupts_init(void)
{
proc_create("interrupts", 0, NULL, &proc_interrupts_operations);
+ kdb_register("interrupts", kdb_interrupts, "",
+ "Show /proc/interrupts", 3);
return 0;
}
fs_initcall(proc_interrupts_init);
--
1.9.0