* [PATCH 1/5] pstore: add tty frontend
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
@ 2023-09-28 2:42 ` Yuanhe Shu
2023-09-28 6:43 ` Greg KH
2023-09-28 2:42 ` [PATCH 2/5] pstore: add multi-backends support Yuanhe Shu
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Yuanhe Shu @ 2023-09-28 2:42 UTC (permalink / raw)
To: gregkh, jirislaby, keescook, tony.luck, gpiccoli, shuah
Cc: linux-kernel, linux-serial, linux-hardening, linux-kselftest,
Yuanhe Shu, Xingrui Yi
Provide a pstore frontend which can log all messages that are send
to tty drivers when there are some problems with drivers or there
is no access to serial ports.
Using pmsg requires us to redirect the output of the user state.
When we need to globally view the serial output of various processes,
it is tedious to redirect the output of each process. We think pmsg is
more suitable for targeted viewing of certain processes' output, and
we also need a tool that can quickly do a global view so that we can
get user-state printed data if the tty driver is working abnormally or
if we don't have serial access.
Furthermore, by enabling tty frontend and console/dmesg frontend in
dump capture kernel, one can collect kernel and user messages to
discover why kdump service works abnormal.
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com>
---
drivers/tty/n_tty.c | 1 +
fs/pstore/Kconfig | 23 +++++++++++++++++
fs/pstore/Makefile | 2 ++
fs/pstore/blk.c | 10 ++++++++
fs/pstore/internal.h | 8 ++++++
fs/pstore/platform.c | 5 ++++
fs/pstore/ram.c | 40 +++++++++++++++++++++++++++--
fs/pstore/tty.c | 50 +++++++++++++++++++++++++++++++++++++
fs/pstore/zone.c | 42 ++++++++++++++++++++++++++++++-
include/linux/pstore.h | 2 ++
include/linux/pstore_blk.h | 3 +++
include/linux/pstore_ram.h | 1 +
include/linux/pstore_zone.h | 2 ++
include/linux/tty.h | 14 +++++++++++
14 files changed, 200 insertions(+), 3 deletions(-)
create mode 100644 fs/pstore/tty.c
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 552e8a741562..55ca40605e4c 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -582,6 +582,7 @@ static ssize_t process_output_block(struct tty_struct *tty,
}
}
break_out:
+ tty_pstore_hook(buf, i);
i = tty->ops->write(tty, buf, i);
mutex_unlock(&ldata->output_lock);
diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index c49d554cc9ae..8a08e4ae71ba 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -147,6 +147,17 @@ config PSTORE_FTRACE
If unsure, say N.
+config PSTORE_TTY
+ bool "Log tty messages"
+ depends on PSTORE
+ help
+ When the option is enabled, pstore will log all messages send
+ to tty drivers, even if no oops or panic happened. It can be
+ useful to log serial information when there is no access to
+ serial ports or the tty drivers work abnormally.
+
+ If unsure, say N.
+
config PSTORE_RAM
tristate "Log panic/oops to a RAM buffer"
depends on PSTORE
@@ -271,3 +282,15 @@ config PSTORE_BLK_FTRACE_SIZE
NOTE that, both Kconfig and module parameters can configure
pstore/blk, but module parameters have priority over Kconfig.
+
+config PSTORE_BLK_TTY_SIZE
+ int "Size in Kbytes of tty log to store"
+ depends on PSTORE_BLK
+ depends on PSTORE_TTY
+ default 64
+ help
+ This just sets size of tty log (tty_size) for pstore/blk. The
+ size is in KB and must be a multiple of 4.
+
+ NOTE that, both Kconfig and module parameters can configure
+ pstore/blk, but module parameters have priority over Kconfig.
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index c270467aeece..4e50efe1ebc8 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -10,6 +10,8 @@ pstore-$(CONFIG_PSTORE_FTRACE) += ftrace.o
pstore-$(CONFIG_PSTORE_PMSG) += pmsg.o
+pstore-$(CONFIG_PSTORE_TTY) += tty.o
+
ramoops-objs += ram.o ram_core.o
obj-$(CONFIG_PSTORE_RAM) += ramoops.o
diff --git a/fs/pstore/blk.c b/fs/pstore/blk.c
index de8cf5d75f34..6b10c6fd7d80 100644
--- a/fs/pstore/blk.c
+++ b/fs/pstore/blk.c
@@ -52,6 +52,14 @@ static long ftrace_size = -1;
module_param(ftrace_size, long, 0400);
MODULE_PARM_DESC(ftrace_size, "ftrace size in kbytes");
+#if IS_ENABLED(CONFIG_PSTORE_TTY)
+static long tty_size = CONFIG_PSTORE_BLK_TTY_SIZE;
+#else
+static long tty_size = -1;
+#endif
+module_param(tty_size, long, 0400);
+MODULE_PARM_DESC(tty_size, "tty_size size in kbytes");
+
static bool best_effort;
module_param(best_effort, bool, 0400);
MODULE_PARM_DESC(best_effort, "use best effort to write (i.e. do not require storage driver pstore support, default: off)");
@@ -130,6 +138,7 @@ static int __register_pstore_device(struct pstore_device_info *dev)
verify_size(pmsg_size, 4096, dev->flags & PSTORE_FLAGS_PMSG);
verify_size(console_size, 4096, dev->flags & PSTORE_FLAGS_CONSOLE);
verify_size(ftrace_size, 4096, dev->flags & PSTORE_FLAGS_FTRACE);
+ verify_size(tty_size, 4096, dev->flags & PSTORE_FLAGS_TTY);
dev->zone.max_reason = max_reason;
/* Initialize required zone ownership details. */
@@ -247,6 +256,7 @@ int pstore_blk_get_config(struct pstore_blk_config *info)
info->pmsg_size = check_size(pmsg_size, 4096);
info->ftrace_size = check_size(ftrace_size, 4096);
info->console_size = check_size(console_size, 4096);
+ info->tty_size = check_size(tty_size, 4096);
return 0;
}
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index 801d6c0b170c..1205366f0523 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -33,6 +33,14 @@ static inline void pstore_register_pmsg(void) {}
static inline void pstore_unregister_pmsg(void) {}
#endif
+#ifdef CONFIG_PSTORE_TTY
+extern void pstore_register_tty(void);
+extern void pstore_unregister_tty(void);
+#else
+static inline void pstore_register_tty(void) {}
+static inline void pstore_unregister_tty(void) {}
+#endif
+
extern struct pstore_info *psinfo;
extern void pstore_set_kmsg_bytes(int);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index cbc0b468c1ab..a6a1df06cfe1 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -58,6 +58,7 @@ static const char * const pstore_type_names[] = {
"powerpc-common",
"pmsg",
"powerpc-opal",
+ "tty",
};
static int pstore_new_entry;
@@ -622,6 +623,8 @@ int pstore_register(struct pstore_info *psi)
pstore_register_ftrace();
if (psi->flags & PSTORE_FLAGS_PMSG)
pstore_register_pmsg();
+ if (psi->flags & PSTORE_FLAGS_TTY)
+ pstore_register_tty();
/* Start watching for new records, if desired. */
pstore_timer_kick();
@@ -662,6 +665,8 @@ void pstore_unregister(struct pstore_info *psi)
pstore_unregister_console();
if (psi->flags & PSTORE_FLAGS_DMESG)
pstore_unregister_kmsg();
+ if (psi->flags & PSTORE_FLAGS_TTY)
+ pstore_unregister_tty();
/* Stop timer and make sure all work has finished. */
del_timer_sync(&pstore_timer);
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 2f625e1fa8d8..f59712bc51d3 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -44,6 +44,10 @@ static ulong ramoops_pmsg_size = MIN_MEM_SIZE;
module_param_named(pmsg_size, ramoops_pmsg_size, ulong, 0400);
MODULE_PARM_DESC(pmsg_size, "size of user space message log");
+static ulong ramoops_tty_size = MIN_MEM_SIZE;
+module_param_named(tty_size, ramoops_tty_size, ulong, 0400);
+MODULE_PARM_DESC(tty_size, "size of tty message log");
+
static unsigned long long mem_address;
module_param_hw(mem_address, ullong, other, 0400);
MODULE_PARM_DESC(mem_address,
@@ -81,6 +85,7 @@ struct ramoops_context {
struct persistent_ram_zone *cprz; /* Console zone */
struct persistent_ram_zone **fprzs; /* Ftrace zones */
struct persistent_ram_zone *mprz; /* PMSG zone */
+ struct persistent_ram_zone *tprz; /* TTY zone */
phys_addr_t phys_addr;
unsigned long size;
unsigned int memtype;
@@ -88,6 +93,7 @@ struct ramoops_context {
size_t console_size;
size_t ftrace_size;
size_t pmsg_size;
+ size_t tty_size;
u32 flags;
struct persistent_ram_ecc_info ecc_info;
unsigned int max_dump_cnt;
@@ -98,6 +104,7 @@ struct ramoops_context {
unsigned int max_ftrace_cnt;
unsigned int ftrace_read_cnt;
unsigned int pmsg_read_cnt;
+ unsigned int tty_read_cnt;
struct pstore_info pstore;
};
@@ -111,6 +118,7 @@ static int ramoops_pstore_open(struct pstore_info *psi)
cxt->console_read_cnt = 0;
cxt->ftrace_read_cnt = 0;
cxt->pmsg_read_cnt = 0;
+ cxt->tty_read_cnt = 0;
return 0;
}
@@ -214,6 +222,9 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
if (!prz_ok(prz) && !cxt->pmsg_read_cnt++)
prz = ramoops_get_next_prz(&cxt->mprz, 0 /* single */, record);
+ if (!prz_ok(prz) && !cxt->tty_read_cnt++)
+ prz = ramoops_get_next_prz(&cxt->tprz, 0 /* single */, record);
+
/* ftrace is last since it may want to dynamically allocate memory. */
if (!prz_ok(prz)) {
if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) &&
@@ -335,6 +346,11 @@ static int notrace ramoops_pstore_write(struct pstore_record *record)
} else if (record->type == PSTORE_TYPE_PMSG) {
pr_warn_ratelimited("PMSG shouldn't call %s\n", __func__);
return -EINVAL;
+ } else if (record->type == PSTORE_TYPE_TTY) {
+ if (!cxt->tprz)
+ return -ENOMEM;
+ persistent_ram_write(cxt->tprz, record->buf, record->size);
+ return 0;
}
if (record->type != PSTORE_TYPE_DMESG)
@@ -426,6 +442,9 @@ static int ramoops_pstore_erase(struct pstore_record *record)
case PSTORE_TYPE_PMSG:
prz = cxt->mprz;
break;
+ case PSTORE_TYPE_TTY:
+ prz = cxt->tprz;
+ break;
default:
return -EINVAL;
}
@@ -458,6 +477,9 @@ static void ramoops_free_przs(struct ramoops_context *cxt)
/* Free console PRZ */
persistent_ram_free(&cxt->cprz);
+ /* Free tty PRZ */
+ persistent_ram_free(&cxt->tprz);
+
/* Free dump PRZs */
if (cxt->dprzs) {
for (i = 0; i < cxt->max_dump_cnt; i++)
@@ -685,6 +707,7 @@ static int ramoops_parse_dt(struct platform_device *pdev,
parse_u32("console-size", pdata->console_size, 0);
parse_u32("ftrace-size", pdata->ftrace_size, 0);
parse_u32("pmsg-size", pdata->pmsg_size, 0);
+ parse_u32("tty-size", pdata->tty_size, 0);
parse_u32("ecc-size", pdata->ecc_info.ecc_size, 0);
parse_u32("flags", pdata->flags, 0);
parse_u32("max-reason", pdata->max_reason, pdata->max_reason);
@@ -705,9 +728,10 @@ static int ramoops_parse_dt(struct platform_device *pdev,
parent_node = of_get_parent(of_node);
if (!of_node_name_eq(parent_node, "reserved-memory") &&
!pdata->console_size && !pdata->ftrace_size &&
- !pdata->pmsg_size && !pdata->ecc_info.ecc_size) {
+ !pdata->pmsg_size && !pdata->tty_size && !pdata->ecc_info.ecc_size) {
pdata->console_size = pdata->record_size;
pdata->pmsg_size = pdata->record_size;
+ pdata->tty_size = pdata->record_size;
}
of_node_put(parent_node);
@@ -765,6 +789,8 @@ static int ramoops_probe(struct platform_device *pdev)
pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size);
if (pdata->pmsg_size && !is_power_of_2(pdata->pmsg_size))
pdata->pmsg_size = rounddown_pow_of_two(pdata->pmsg_size);
+ if (pdata->tty_size && !is_power_of_2(pdata->tty_size))
+ pdata->tty_size = rounddown_pow_of_two(pdata->tty_size);
cxt->size = pdata->mem_size;
cxt->phys_addr = pdata->mem_address;
@@ -773,13 +799,14 @@ static int ramoops_probe(struct platform_device *pdev)
cxt->console_size = pdata->console_size;
cxt->ftrace_size = pdata->ftrace_size;
cxt->pmsg_size = pdata->pmsg_size;
+ cxt->tty_size = pdata->tty_size;
cxt->flags = pdata->flags;
cxt->ecc_info = pdata->ecc_info;
paddr = cxt->phys_addr;
dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size
- - cxt->pmsg_size;
+ - cxt->pmsg_size - cxt->tty_size;
err = ramoops_init_przs("dmesg", dev, cxt, &cxt->dprzs, &paddr,
dump_mem_sz, cxt->record_size,
&cxt->max_dump_cnt, 0, 0);
@@ -796,6 +823,11 @@ static int ramoops_probe(struct platform_device *pdev)
if (err)
goto fail_init;
+ err = ramoops_init_prz("tty", dev, cxt, &cxt->tprz, &paddr,
+ cxt->tty_size, 0);
+ if (err)
+ goto fail_init;
+
cxt->max_ftrace_cnt = (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)
? nr_cpu_ids
: 1;
@@ -825,6 +857,8 @@ static int ramoops_probe(struct platform_device *pdev)
cxt->pstore.flags |= PSTORE_FLAGS_FTRACE;
if (cxt->pmsg_size)
cxt->pstore.flags |= PSTORE_FLAGS_PMSG;
+ if (cxt->tty_size)
+ cxt->pstore.flags |= PSTORE_FLAGS_TTY;
/*
* Since bufsize is only used for dmesg crash dumps, it
@@ -858,6 +892,7 @@ static int ramoops_probe(struct platform_device *pdev)
ramoops_console_size = pdata->console_size;
ramoops_pmsg_size = pdata->pmsg_size;
ramoops_ftrace_size = pdata->ftrace_size;
+ ramoops_tty_size = pdata->tty_size;
pr_info("using 0x%lx@0x%llx, ecc: %d\n",
cxt->size, (unsigned long long)cxt->phys_addr,
@@ -929,6 +964,7 @@ static void __init ramoops_register_dummy(void)
pdata.console_size = ramoops_console_size;
pdata.ftrace_size = ramoops_ftrace_size;
pdata.pmsg_size = ramoops_pmsg_size;
+ pdata.tty_size = ramoops_tty_size;
/* If "max_reason" is set, its value has priority over "dump_oops". */
if (ramoops_max_reason >= 0)
pdata.max_reason = ramoops_max_reason;
diff --git a/fs/pstore/tty.c b/fs/pstore/tty.c
new file mode 100644
index 000000000000..432ed7194188
--- /dev/null
+++ b/fs/pstore/tty.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Provide a pstore frontend which can log all messages that are send
+ * to tty drivers when there are some problems with drivers or there
+ * is no access to serial ports.
+ */
+
+#include <linux/kernel.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include "internal.h"
+
+DEFINE_STATIC_KEY_FALSE(tty_key);
+
+#define TTY_NAME "tty"
+#undef pr_fmt
+#define pr_fmt(fmt) TTY_NAME ": " fmt
+
+static void do_write_ttymsg(const unsigned char *buf, int count,
+ struct pstore_info *psinfo)
+{
+ struct pstore_record record, newline;
+
+ pstore_record_init(&record, psinfo);
+ record.type = PSTORE_TYPE_TTY;
+ record.size = count;
+ record.buf = (char *)buf;
+ psinfo->write(&record);
+
+ pstore_record_init(&newline, psinfo);
+ newline.type = PSTORE_TYPE_TTY;
+ newline.size = strlen("\n");
+ newline.buf = "\n";
+ psinfo->write(&newline);
+}
+
+void pstore_register_tty(void)
+{
+ static_branch_enable(&tty_key);
+}
+
+void pstore_start_tty(const unsigned char *buf, int count)
+{
+ do_write_ttymsg(buf, count);
+}
+
+void pstore_unregister_tty(void)
+{
+ static_branch_disable(&tty_key);
+}
diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index 2770746bb7aa..ca985e9493d7 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -93,6 +93,7 @@ struct pstore_zone {
* @ppsz: pmsg storage zone
* @cpsz: console storage zone
* @fpszs: ftrace storage zones
+ * @tpsz: tty storage zone
* @kmsg_max_cnt: max count of @kpszs
* @kmsg_read_cnt: counter of total read kmsg dumps
* @kmsg_write_cnt: counter of total kmsg dump writes
@@ -100,6 +101,7 @@ struct pstore_zone {
* @console_read_cnt: counter of total read console zone
* @ftrace_max_cnt: max count of @fpszs
* @ftrace_read_cnt: counter of max read ftrace zone
+ * @tty_read_cnt: counter of total read tty zone
* @oops_counter: counter of oops dumps
* @panic_counter: counter of panic dumps
* @recovered: whether finished recovering data from storage
@@ -113,6 +115,7 @@ struct psz_context {
struct pstore_zone *ppsz;
struct pstore_zone *cpsz;
struct pstore_zone **fpszs;
+ struct pstore_zone *tpsz;
unsigned int kmsg_max_cnt;
unsigned int kmsg_read_cnt;
unsigned int kmsg_write_cnt;
@@ -120,6 +123,7 @@ struct psz_context {
unsigned int console_read_cnt;
unsigned int ftrace_max_cnt;
unsigned int ftrace_read_cnt;
+ unsigned int tty_read_cnt;
/*
* These counters should be calculated during recovery.
* It records the oops/panic times after crashes rather than boots.
@@ -325,6 +329,8 @@ static void psz_flush_all_dirty_zones(struct work_struct *work)
ret |= psz_flush_dirty_zones(cxt->kpszs, cxt->kmsg_max_cnt);
if (cxt->fpszs)
ret |= psz_flush_dirty_zones(cxt->fpszs, cxt->ftrace_max_cnt);
+ if (cxt->tpsz)
+ ret |= psz_flush_dirty_zone(cxt->tpsz);
if (ret && cxt->pstore_zone_info)
schedule_delayed_work(&psz_cleaner, msecs_to_jiffies(1000));
}
@@ -617,6 +623,10 @@ static inline int psz_recovery(struct psz_context *cxt)
if (ret)
goto out;
+ ret = psz_recover_zone(cxt, cxt->tpsz);
+ if (ret)
+ goto out;
+
ret = psz_recover_zones(cxt, cxt->fpszs, cxt->ftrace_max_cnt);
out:
@@ -637,6 +647,7 @@ static int psz_pstore_open(struct pstore_info *psi)
cxt->pmsg_read_cnt = 0;
cxt->console_read_cnt = 0;
cxt->ftrace_read_cnt = 0;
+ cxt->tty_read_cnt = 0;
return 0;
}
@@ -713,6 +724,8 @@ static int psz_pstore_erase(struct pstore_record *record)
if (record->id >= cxt->ftrace_max_cnt)
return -EINVAL;
return psz_record_erase(cxt, cxt->fpszs[record->id]);
+ case PSTORE_TYPE_TTY:
+ return psz_record_erase(cxt, cxt->tpsz);
default: return -EINVAL;
}
}
@@ -891,6 +904,8 @@ static int notrace psz_pstore_write(struct pstore_record *record)
return psz_record_write(cxt->cpsz, record);
case PSTORE_TYPE_PMSG:
return psz_record_write(cxt->ppsz, record);
+ case PSTORE_TYPE_TTY:
+ return psz_record_write(cxt->tpsz, record);
case PSTORE_TYPE_FTRACE: {
int zonenum = smp_processor_id();
@@ -935,6 +950,13 @@ static struct pstore_zone *psz_read_next_zone(struct psz_context *cxt)
return zone;
}
+ if (cxt->tty_read_cnt == 0) {
+ cxt->tty_read_cnt++;
+ zone = cxt->tpsz;
+ if (psz_old_ok(zone))
+ return zone;
+ }
+
return NULL;
}
@@ -1081,6 +1103,7 @@ static ssize_t psz_pstore_read(struct pstore_record *record)
readop = psz_ftrace_read;
break;
case PSTORE_TYPE_CONSOLE:
+ case PSTORE_TYPE_TTY:
case PSTORE_TYPE_PMSG:
readop = psz_record_read;
break;
@@ -1145,6 +1168,8 @@ static void psz_free_all_zones(struct psz_context *cxt)
psz_free_zone(&cxt->cpsz);
if (cxt->fpszs)
psz_free_zones(&cxt->fpszs, &cxt->ftrace_max_cnt);
+ if (cxt->tpsz)
+ psz_free_zone(&cxt->tpsz);
}
static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
@@ -1255,6 +1280,15 @@ static int psz_alloc_zones(struct psz_context *cxt)
goto free_out;
}
+ off_size += info->tty_size;
+ cxt->tpsz = psz_init_zone(PSTORE_TYPE_TTY, &off,
+ info->tty_size);
+ if (IS_ERR(cxt->tpsz)) {
+ err = PTR_ERR(cxt->tpsz);
+ cxt->tpsz = NULL;
+ goto free_out;
+ }
+
off_size += info->ftrace_size;
cxt->fpszs = psz_init_zones(PSTORE_TYPE_FTRACE, &off,
info->ftrace_size,
@@ -1305,7 +1339,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
}
if (!info->kmsg_size && !info->pmsg_size && !info->console_size &&
- !info->ftrace_size) {
+ !info->ftrace_size && !info->tty_size) {
pr_warn("at least one record size must be non-zero\n");
return -EINVAL;
}
@@ -1330,6 +1364,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
check_size(pmsg_size, SECTOR_SIZE);
check_size(console_size, SECTOR_SIZE);
check_size(ftrace_size, SECTOR_SIZE);
+ check_size(tty_size, SECTOR_SIZE);
#undef check_size
@@ -1358,6 +1393,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
pr_debug("\tpmsg size : %ld Bytes\n", info->pmsg_size);
pr_debug("\tconsole size : %ld Bytes\n", info->console_size);
pr_debug("\tftrace size : %ld Bytes\n", info->ftrace_size);
+ pr_debug("\ttty size : %ld Bytes\n", info->tty_size);
err = psz_alloc_zones(cxt);
if (err) {
@@ -1399,6 +1435,10 @@ int register_pstore_zone(struct pstore_zone_info *info)
cxt->pstore.flags |= PSTORE_FLAGS_FTRACE;
pr_cont(" ftrace");
}
+ if (info->tty_size) {
+ cxt->pstore.flags |= PSTORE_FLAGS_TTY;
+ pr_cont(" tty");
+ }
pr_cont("\n");
err = pstore_register(&cxt->pstore);
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 638507a3c8ff..791c86552921 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -38,6 +38,7 @@ enum pstore_type_id {
PSTORE_TYPE_PPC_COMMON = 6,
PSTORE_TYPE_PMSG = 7,
PSTORE_TYPE_PPC_OPAL = 8,
+ PSTORE_TYPE_TTY = 9,
/* End of the list */
PSTORE_TYPE_MAX
@@ -206,6 +207,7 @@ struct pstore_info {
#define PSTORE_FLAGS_CONSOLE BIT(1)
#define PSTORE_FLAGS_FTRACE BIT(2)
#define PSTORE_FLAGS_PMSG BIT(3)
+#define PSTORE_FLAGS_TTY BIT(4)
extern int pstore_register(struct pstore_info *);
extern void pstore_unregister(struct pstore_info *);
diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 924ca07aafbd..8cebd317527f 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -33,6 +33,7 @@ void unregister_pstore_device(struct pstore_device_info *dev);
* @pmsg_size: Total size of the pmsg storage area
* @console_size: Total size of the console storage area
* @ftrace_size: Total size for ftrace logging data (for all CPUs)
+ * @tty_size: Total size of the tty storage area
*/
struct pstore_blk_config {
char device[80];
@@ -41,6 +42,8 @@ struct pstore_blk_config {
unsigned long pmsg_size;
unsigned long console_size;
unsigned long ftrace_size;
+ unsigned long tty_size;
+
};
/**
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 9d65ff94e216..ad343e36ce18 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -34,6 +34,7 @@ struct ramoops_platform_data {
unsigned long console_size;
unsigned long ftrace_size;
unsigned long pmsg_size;
+ unsigned long tty_size;
int max_reason;
u32 flags;
struct persistent_ram_ecc_info ecc_info;
diff --git a/include/linux/pstore_zone.h b/include/linux/pstore_zone.h
index 1e35eaa33e5e..80d95e02750f 100644
--- a/include/linux/pstore_zone.h
+++ b/include/linux/pstore_zone.h
@@ -21,6 +21,7 @@ typedef ssize_t (*pstore_zone_erase_op)(size_t, loff_t);
* @pmsg_size: The size of pmsg zone which is the same as @kmsg_size.
* @console_size:The size of console zone which is the same as @kmsg_size.
* @ftrace_size:The size of ftrace zone which is the same as @kmsg_size.
+ * @tty_size:The size of tty zone which is the same as @kmsg_size.
* @read: The general read operation. Both of the function parameters
* @size and @offset are relative value to storage.
* On success, the number of bytes should be returned, others
@@ -48,6 +49,7 @@ struct pstore_zone_info {
unsigned long pmsg_size;
unsigned long console_size;
unsigned long ftrace_size;
+ unsigned long tty_size;
pstore_zone_read_op read;
pstore_zone_write_op write;
pstore_zone_erase_op erase;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index e8d5d9997aca..88c9412d671a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -434,6 +434,20 @@ void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud,
void tty_encode_baud_rate(struct tty_struct *tty, speed_t ibaud,
speed_t obaud);
+#ifdef CONFIG_PSTORE_TTY
+DECLARE_STATIC_KEY_FALSE(tty_key);
+extern void pstore_start_tty(const unsigned char *buf, int count);
+
+static inline void tty_pstore_hook(const unsigned char *buf, int count)
+{
+ if (static_branch_unlikely(&tty_key))
+ pstore_start_tty(buf, count);
+}
+
+#else
+static inline void tty_pstore_hook(const unsigned char *buf, int count) {}
+#endif
+
/**
* tty_get_baud_rate - get tty bit rates
* @tty: tty to query
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] pstore: add tty frontend
2023-09-28 2:42 ` [PATCH 1/5] pstore: add tty frontend Yuanhe Shu
@ 2023-09-28 6:43 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2023-09-28 6:43 UTC (permalink / raw)
To: Yuanhe Shu
Cc: jirislaby, keescook, tony.luck, gpiccoli, shuah, linux-kernel,
linux-serial, linux-hardening, linux-kselftest, Xingrui Yi
On Thu, Sep 28, 2023 at 10:42:40AM +0800, Yuanhe Shu wrote:
> Provide a pstore frontend which can log all messages that are send
> to tty drivers when there are some problems with drivers or there
> is no access to serial ports.
>
> Using pmsg requires us to redirect the output of the user state.
> When we need to globally view the serial output of various processes,
> it is tedious to redirect the output of each process. We think pmsg is
> more suitable for targeted viewing of certain processes' output, and
> we also need a tool that can quickly do a global view so that we can
> get user-state printed data if the tty driver is working abnormally or
> if we don't have serial access.
>
> Furthermore, by enabling tty frontend and console/dmesg frontend in
> dump capture kernel, one can collect kernel and user messages to
> discover why kdump service works abnormal.
>
> Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
> Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com>
> ---
> drivers/tty/n_tty.c | 1 +
> fs/pstore/Kconfig | 23 +++++++++++++++++
> fs/pstore/Makefile | 2 ++
> fs/pstore/blk.c | 10 ++++++++
> fs/pstore/internal.h | 8 ++++++
> fs/pstore/platform.c | 5 ++++
> fs/pstore/ram.c | 40 +++++++++++++++++++++++++++--
> fs/pstore/tty.c | 50 +++++++++++++++++++++++++++++++++++++
> fs/pstore/zone.c | 42 ++++++++++++++++++++++++++++++-
> include/linux/pstore.h | 2 ++
> include/linux/pstore_blk.h | 3 +++
> include/linux/pstore_ram.h | 1 +
> include/linux/pstore_zone.h | 2 ++
> include/linux/tty.h | 14 +++++++++++
> 14 files changed, 200 insertions(+), 3 deletions(-)
> create mode 100644 fs/pstore/tty.c
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 552e8a741562..55ca40605e4c 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -582,6 +582,7 @@ static ssize_t process_output_block(struct tty_struct *tty,
> }
> }
> break_out:
> + tty_pstore_hook(buf, i);
"hook"? That does not help explain what this is doing here at all. A
better name would be best, but why is this part of n_tty at all? Why
isn't it a console driver just for this, you are sitting in the middle
of the fast-path of the tty layer sucking in data all the time, what is
the performance impact of this extra jump?
> --- a/fs/pstore/blk.c
> +++ b/fs/pstore/blk.c
> @@ -52,6 +52,14 @@ static long ftrace_size = -1;
> module_param(ftrace_size, long, 0400);
> MODULE_PARM_DESC(ftrace_size, "ftrace size in kbytes");
>
> +#if IS_ENABLED(CONFIG_PSTORE_TTY)
> +static long tty_size = CONFIG_PSTORE_BLK_TTY_SIZE;
> +#else
> +static long tty_size = -1;
> +#endif
> +module_param(tty_size, long, 0400);
> +MODULE_PARM_DESC(tty_size, "tty_size size in kbytes");
Do we really need more module parameters that are undocumented?
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 2f625e1fa8d8..f59712bc51d3 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -44,6 +44,10 @@ static ulong ramoops_pmsg_size = MIN_MEM_SIZE;
> module_param_named(pmsg_size, ramoops_pmsg_size, ulong, 0400);
> MODULE_PARM_DESC(pmsg_size, "size of user space message log");
>
> +static ulong ramoops_tty_size = MIN_MEM_SIZE;
> +module_param_named(tty_size, ramoops_tty_size, ulong, 0400);
> +MODULE_PARM_DESC(tty_size, "size of tty message log");
Again, why a module parameter?
> --- /dev/null
> +++ b/fs/pstore/tty.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Provide a pstore frontend which can log all messages that are send
> + * to tty drivers when there are some problems with drivers or there
> + * is no access to serial ports.
> + */
No copyright line? Are you sure your company is ok with that?
> +
> +#include <linux/kernel.h>
> +#include <linux/tty.h>
> +#include <linux/tty_driver.h>
> +#include "internal.h"
> +
> +DEFINE_STATIC_KEY_FALSE(tty_key);
> +
> +#define TTY_NAME "tty"
> +#undef pr_fmt
> +#define pr_fmt(fmt) TTY_NAME ": " fmt
> +
> +static void do_write_ttymsg(const unsigned char *buf, int count,
> + struct pstore_info *psinfo)
Odd formatting :(
> +{
> + struct pstore_record record, newline;
> +
> + pstore_record_init(&record, psinfo);
> + record.type = PSTORE_TYPE_TTY;
> + record.size = count;
> + record.buf = (char *)buf;
Why the casting? Why isn't the buffer a u8 * here?
> + psinfo->write(&record);
> +
> + pstore_record_init(&newline, psinfo);
> + newline.type = PSTORE_TYPE_TTY;
> + newline.size = strlen("\n");
> + newline.buf = "\n";
> + psinfo->write(&newline);
Why have 2 structures and not just one? And why the new line all the
time, you are not guaranteed that your buffer must have a new line here,
it could be in the middle of a hunk of data that is longer than the
normal buffer size. Why not rely on the tty data to handle the new line
stuff for you in the proper location?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] pstore: add multi-backends support
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
2023-09-28 2:42 ` [PATCH 1/5] pstore: add tty frontend Yuanhe Shu
@ 2023-09-28 2:42 ` Yuanhe Shu
2023-09-28 2:42 ` [PATCH 3/5] pstore: add subdirs for multi-backends Yuanhe Shu
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Yuanhe Shu @ 2023-09-28 2:42 UTC (permalink / raw)
To: gregkh, jirislaby, keescook, tony.luck, gpiccoli, shuah
Cc: linux-kernel, linux-serial, linux-hardening, linux-kselftest,
Yuanhe Shu, Xingrui Yi
Currently, pstore supports only one backend open at a time.
Specifically, due to the global variable "psinfo", pstore only accepts
the first registered backend. If a new backend wants to register later,
pstore will simply reject it and return an error. This design forced us
to close existing backend in order to use the new ones.
To enable pstore to support multiple backends, "psinfo" is replaced by
"psinfo_list", a list that holds multiple "psinfo". If multiple backends
are registered with the same frontend, the frontend is reused.
Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com>
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
fs/pstore/ftrace.c | 22 +++-
fs/pstore/inode.c | 16 +--
fs/pstore/internal.h | 7 +-
fs/pstore/platform.c | 224 ++++++++++++++++++++++++++---------------
fs/pstore/pmsg.c | 23 ++++-
fs/pstore/tty.c | 8 +-
include/linux/pstore.h | 29 ++++++
7 files changed, 232 insertions(+), 97 deletions(-)
diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
index 776cae20af4e..d3176fb94c95 100644
--- a/fs/pstore/ftrace.c
+++ b/fs/pstore/ftrace.c
@@ -23,10 +23,11 @@
/* This doesn't need to be atomic: speed is chosen over correctness here. */
static u64 pstore_ftrace_stamp;
-static void notrace pstore_ftrace_call(unsigned long ip,
+static void notrace pstore_do_ftrace(unsigned long ip,
unsigned long parent_ip,
struct ftrace_ops *op,
- struct ftrace_regs *fregs)
+ struct ftrace_regs *fregs,
+ struct ftrace_info *psinfo)
{
int bit;
unsigned long flags;
@@ -57,6 +58,20 @@ static void notrace pstore_ftrace_call(unsigned long ip,
ftrace_test_recursion_unlock(bit);
}
+static void notrace pstore_ftrace_call(unsigned long ip,
+ unsigned long parent_ip,
+ struct ftrace_ops *op,
+ struct pt_regs *regs)
+{
+ struct pstore_info_list *entry;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_FTRACE)
+ pstore_do_ftrace(ip, parent_ip, op, regs, entry->psi);
+ rcu_read_unlock();
+}
+
static struct ftrace_ops pstore_ftrace_ops __read_mostly = {
.func = pstore_ftrace_call,
};
@@ -131,9 +146,6 @@ MODULE_PARM_DESC(record_ftrace,
void pstore_register_ftrace(void)
{
- if (!psinfo->write)
- return;
-
pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
pstore_set_ftrace_enabled(record_ftrace);
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index ffbadb8b3032..312e7d55b95f 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -288,7 +288,7 @@ static const struct super_operations pstore_ops = {
.show_options = pstore_show_options,
};
-static struct dentry *psinfo_lock_root(void)
+static struct dentry *psinfo_lock_root(struct pstore_info *psinfo)
{
struct dentry *root;
@@ -315,7 +315,7 @@ int pstore_put_backend_records(struct pstore_info *psi)
struct dentry *root;
int rc = 0;
- root = psinfo_lock_root();
+ root = psinfo_lock_root(psi);
if (!root)
return 0;
@@ -414,21 +414,22 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
* when we are re-scanning the backing store looking to add new
* error records.
*/
-void pstore_get_records(int quiet)
+void pstore_get_records(struct pstore_info *psi, int pos, int quiet)
{
struct dentry *root;
- root = psinfo_lock_root();
+ root = psinfo_lock_root(psi);
if (!root)
return;
- pstore_get_backend_records(psinfo, root, quiet);
+ pstore_get_backend_records(psi, root, quiet, pos);
inode_unlock(d_inode(root));
}
static int pstore_fill_super(struct super_block *sb, void *data, int silent)
{
struct inode *inode;
+ struct pstore_info_list *entry;
sb->s_maxbytes = MAX_LFS_FILESIZE;
sb->s_blocksize = PAGE_SIZE;
@@ -454,7 +455,10 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
pstore_sb = sb;
mutex_unlock(&pstore_sb_lock);
- pstore_get_records(0);
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ pstore_get_records(entry->psi, entry->index, 0);
+ rcu_read_unlock();
return 0;
}
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index 1205366f0523..7465b5baf002 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -41,12 +41,13 @@ static inline void pstore_register_tty(void) {}
static inline void pstore_unregister_tty(void) {}
#endif
-extern struct pstore_info *psinfo;
+extern struct pstore_backends *psback;
extern void pstore_set_kmsg_bytes(int);
-extern void pstore_get_records(int);
+extern void pstore_get_records(struct pstore_info *psi, int pos,
+ int quiet);
extern void pstore_get_backend_records(struct pstore_info *psi,
- struct dentry *root, int quiet);
+ struct dentry *root, int quiet, int pos);
extern int pstore_put_backend_records(struct pstore_info *psi);
extern int pstore_mkfile(struct dentry *root,
struct pstore_record *record);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index a6a1df06cfe1..34fb21e6ff4f 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -70,12 +70,12 @@ static void pstore_dowork(struct work_struct *);
static DECLARE_WORK(pstore_work, pstore_dowork);
/*
- * psinfo_lock protects "psinfo" during calls to
+ * psback_lock protects "psback" during calls to
* pstore_register(), pstore_unregister(), and
* the filesystem mount/unmount routines.
*/
-static DEFINE_MUTEX(psinfo_lock);
-struct pstore_info *psinfo;
+DEFINE_MUTEX(psback_lock);
+struct pstore_backends *psback;
static char *backend;
module_param(backend, charp, 0444);
@@ -103,8 +103,8 @@ struct pstore_zbackend {
const char *name;
};
-static char *big_oops_buf;
-static size_t big_oops_buf_sz;
+static char *big_oops_buf[PSTORE_BACKEND_NUM];
+static size_t big_oops_buf_sz[PSTORE_BACKEND_NUM];
void pstore_set_kmsg_bytes(int bytes)
{
@@ -285,7 +285,7 @@ static int pstore_compress(const void *in, void *out,
return outlen;
}
-static void allocate_buf_for_compression(void)
+static void allocate_buf_for_compression(struct pstore_info *psinfo, int pos)
{
struct crypto_comp *ctx;
int size;
@@ -295,8 +295,8 @@ static void allocate_buf_for_compression(void)
if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !zbackend)
return;
- /* Skip if no pstore backend yet or compression init already done. */
- if (!psinfo || tfm)
+ /* Skip if no pstore backend yet. */
+ if (!psinfo)
return;
if (!crypto_has_comp(zbackend->name, 0, 0)) {
@@ -328,21 +328,21 @@ static void allocate_buf_for_compression(void)
/* A non-NULL big_oops_buf indicates compression is available. */
tfm = ctx;
- big_oops_buf_sz = size;
- big_oops_buf = buf;
+ big_oops_buf_sz[pos] = size;
+ big_oops_buf[pos] = buf;
pr_info("Using crash dump compression: %s\n", zbackend->name);
}
-static void free_buf_for_compression(void)
+static void free_buf_for_compression(int pos)
{
if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && tfm) {
crypto_free_comp(tfm);
tfm = NULL;
}
- kfree(big_oops_buf);
- big_oops_buf = NULL;
- big_oops_buf_sz = 0;
+ kfree(big_oops_buf[pos]);
+ big_oops_buf[pos] = NULL;
+ big_oops_buf_sz[pos] = 0;
}
/*
@@ -352,7 +352,8 @@ static void free_buf_for_compression(void)
* printk buffer which results in fetching old contents.
* Copy the recent messages from big_oops_buf to psinfo->buf
*/
-static size_t copy_kmsg_to_buffer(int hsize, size_t len)
+static size_t copy_kmsg_to_buffer(int hsize, size_t len,
+ struct pstore_info *psinfo, int pos)
{
size_t total_len;
size_t diff;
@@ -361,12 +362,12 @@ static size_t copy_kmsg_to_buffer(int hsize, size_t len)
if (total_len > psinfo->bufsize) {
diff = total_len - psinfo->bufsize + hsize;
- memcpy(psinfo->buf, big_oops_buf, hsize);
- memcpy(psinfo->buf + hsize, big_oops_buf + diff,
+ memcpy(psinfo->buf, big_oops_buf[pos], hsize);
+ memcpy(psinfo->buf + hsize, big_oops_buf[pos] + diff,
psinfo->bufsize - hsize);
total_len = psinfo->bufsize;
} else
- memcpy(psinfo->buf, big_oops_buf, total_len);
+ memcpy(psinfo->buf, big_oops_buf[pos], total_len);
return total_len;
}
@@ -386,8 +387,8 @@ void pstore_record_init(struct pstore_record *record,
* callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the
* end of the buffer.
*/
-static void pstore_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+static void pstore_do_dump(struct kmsg_dumper *dumper,
+ enum kmsg_dump_reason reason, struct pstore_info *psinfo, int pos)
{
struct kmsg_dump_iter iter;
unsigned long total = 0;
@@ -427,9 +428,9 @@ static void pstore_dump(struct kmsg_dumper *dumper,
record.part = part;
record.buf = psinfo->buf;
- if (big_oops_buf) {
- dst = big_oops_buf;
- dst_size = big_oops_buf_sz;
+ if (big_oops_buf[pos]) {
+ dst = big_oops_buf[pos];
+ dst_size = big_oops_buf_sz[pos];
} else {
dst = psinfo->buf;
dst_size = psinfo->bufsize;
@@ -445,7 +446,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
dst_size, &dump_size))
break;
- if (big_oops_buf) {
+ if (big_oops_buf[pos]) {
zipped_len = pstore_compress(dst, psinfo->buf,
header_size + dump_size,
psinfo->bufsize);
@@ -455,7 +456,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
record.size = zipped_len;
} else {
record.size = copy_kmsg_to_buffer(header_size,
- dump_size);
+ dump_size, psinfo, pos);
}
} else {
record.size = header_size + dump_size;
@@ -482,6 +483,18 @@ static void pstore_dump(struct kmsg_dumper *dumper,
}
}
+static void pstore_dump(struct kmsg_dumper *dumper,
+ enum kmsg_dump_reason reason)
+{
+ struct pstore_info_list *entry;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_DMESG)
+ pstore_do_dump(dumper, reason, entry->psi, entry->index);
+ rcu_read_unlock();
+}
+
static struct kmsg_dumper pstore_dumper = {
.dump = pstore_dump,
};
@@ -500,13 +513,11 @@ static void pstore_unregister_kmsg(void)
}
#ifdef CONFIG_PSTORE_CONSOLE
-static void pstore_console_write(struct console *con, const char *s, unsigned c)
+static void pstore_console_do_write(struct console *con, const char *s,
+ unsigned int c, struct pstore_info *psinfo)
{
struct pstore_record record;
- if (!c)
- return;
-
pstore_record_init(&record, psinfo);
record.type = PSTORE_TYPE_CONSOLE;
@@ -515,6 +526,20 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c)
psinfo->write(&record);
}
+static void pstore_console_write(struct console *con, const char *s, unsigned int c)
+{
+ struct pstore_info_list *entry;
+
+ if (!c)
+ return;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_CONSOLE)
+ pstore_console_do_write(con, s, c, entry->psi);
+ rcu_read_unlock();
+}
+
static struct console pstore_console = {
.write = pstore_console_write,
.index = -1,
@@ -523,7 +548,7 @@ static struct console pstore_console = {
static void pstore_register_console(void)
{
/* Show which backend is going to get console writes. */
- strscpy(pstore_console.name, psinfo->name,
+ strscpy(pstore_console.name, "pstore console",
sizeof(pstore_console.name));
/*
* Always initialize flags here since prior unregister_console()
@@ -574,11 +599,8 @@ static int pstore_write_user_compat(struct pstore_record *record,
*/
int pstore_register(struct pstore_info *psi)
{
- if (backend && strcmp(backend, psi->name)) {
- pr_warn("backend '%s' already in use: ignoring '%s'\n",
- backend, psi->name);
- return -EBUSY;
- }
+ struct pstore_info_list *entry;
+ struct pstore_info_list *newpsi;
/* Sanity check flags. */
if (!psi->flags) {
@@ -594,36 +616,57 @@ int pstore_register(struct pstore_info *psi)
return -EINVAL;
}
- mutex_lock(&psinfo_lock);
- if (psinfo) {
- pr_warn("backend '%s' already loaded: ignoring '%s'\n",
- psinfo->name, psi->name);
- mutex_unlock(&psinfo_lock);
- return -EBUSY;
+ mutex_lock(&psback_lock);
+
+ if (psback) {
+ if (psback->flag == PSTORE_LIST_FULL) {
+ pr_warn("registration space is used up: ignoring '%s'\n", psi->name);
+ mutex_unlock(&psback_lock);
+ return -EBUSY;
+ }
+ list_for_each_entry(entry, &psback->list_entry, list) {
+ if (strcmp(entry->psi->name, psi->name) == 0) {
+ pr_warn("backend '%s' already loaded\n", psi->name);
+ mutex_unlock(&psback_lock);
+ return -EPERM;
+ }
+ }
+ }
+
+ if (!psback) {
+ psback = kzalloc(sizeof(*psback), GFP_KERNEL);
+ INIT_LIST_HEAD(&psback->list_entry);
}
if (!psi->write_user)
psi->write_user = pstore_write_user_compat;
- psinfo = psi;
- mutex_init(&psinfo->read_mutex);
- spin_lock_init(&psinfo->buf_lock);
+ newpsi = kzalloc(sizeof(*newpsi), GFP_KERNEL);
+ newpsi->psi = psi;
+ newpsi->index = ffz(psback->flag);
+ INIT_LIST_HEAD(&newpsi->list);
+ psback->flag |= (1 << newpsi->index);
+
+ mutex_init(&psi->read_mutex);
+ spin_lock_init(&psi->buf_lock);
+
+ if (psi->flags & PSTORE_FLAGS_DMESG && !psback->front_cnt[PSTORE_TYPE_DMESG])
+ allocate_buf_for_compression(psi, newpsi->index);
- if (psi->flags & PSTORE_FLAGS_DMESG)
- allocate_buf_for_compression();
+ pstore_get_records(psi, newpsi->index, 0);
- pstore_get_records(0);
+ list_add_rcu(&newpsi->list, &psback->list_entry);
- if (psi->flags & PSTORE_FLAGS_DMESG) {
- pstore_dumper.max_reason = psinfo->max_reason;
+ if (psi->flags & PSTORE_FLAGS_DMESG && !psback->front_cnt[PSTORE_TYPE_DMESG]++) {
+ pstore_dumper.max_reason = psi->max_reason;
pstore_register_kmsg();
}
- if (psi->flags & PSTORE_FLAGS_CONSOLE)
+ if (psi->flags & PSTORE_FLAGS_CONSOLE && !psback->front_cnt[PSTORE_TYPE_CONSOLE]++)
pstore_register_console();
- if (psi->flags & PSTORE_FLAGS_FTRACE)
+ if (psi->flags & PSTORE_FLAGS_FTRACE && !psback->front_cnt[PSTORE_TYPE_FTRACE]++)
pstore_register_ftrace();
- if (psi->flags & PSTORE_FLAGS_PMSG)
+ if (psi->flags & PSTORE_FLAGS_PMSG && !psback->front_cnt[PSTORE_TYPE_PMSG]++)
pstore_register_pmsg();
- if (psi->flags & PSTORE_FLAGS_TTY)
+ if (psi->flags & PSTORE_FLAGS_TTY && !psback->front_cnt[PSTORE_TYPE_TTY]++)
pstore_register_tty();
/* Start watching for new records, if desired. */
@@ -637,35 +680,30 @@ int pstore_register(struct pstore_info *psi)
pr_info("Registered %s as persistent store backend\n", psi->name);
- mutex_unlock(&psinfo_lock);
+ mutex_unlock(&psback_lock);
return 0;
}
EXPORT_SYMBOL_GPL(pstore_register);
void pstore_unregister(struct pstore_info *psi)
{
+ struct pstore_info_list *entry;
/* It's okay to unregister nothing. */
if (!psi)
return;
- mutex_lock(&psinfo_lock);
-
- /* Only one backend can be registered at a time. */
- if (WARN_ON(psi != psinfo)) {
- mutex_unlock(&psinfo_lock);
- return;
- }
+ mutex_lock(&psback_lock);
/* Unregister all callbacks. */
- if (psi->flags & PSTORE_FLAGS_PMSG)
+ if (psi->flags & PSTORE_FLAGS_PMSG && !--psback->front_cnt[PSTORE_TYPE_PMSG])
pstore_unregister_pmsg();
- if (psi->flags & PSTORE_FLAGS_FTRACE)
+ if (psi->flags & PSTORE_FLAGS_FTRACE && !--psback->front_cnt[PSTORE_TYPE_FTRACE])
pstore_unregister_ftrace();
- if (psi->flags & PSTORE_FLAGS_CONSOLE)
+ if (psi->flags & PSTORE_FLAGS_CONSOLE && !--psback->front_cnt[PSTORE_TYPE_CONSOLE])
pstore_unregister_console();
- if (psi->flags & PSTORE_FLAGS_DMESG)
+ if (psi->flags & PSTORE_FLAGS_DMESG && !--psback->front_cnt[PSTORE_TYPE_DMESG])
pstore_unregister_kmsg();
- if (psi->flags & PSTORE_FLAGS_TTY)
+ if (psi->flags & PSTORE_FLAGS_TTY && !--psback->front_cnt[PSTORE_TYPE_TTY])
pstore_unregister_tty();
/* Stop timer and make sure all work has finished. */
@@ -675,18 +713,28 @@ void pstore_unregister(struct pstore_info *psi)
/* Remove all backend records from filesystem tree. */
pstore_put_backend_records(psi);
- free_buf_for_compression();
+ list_for_each_entry(entry, &psback->list_entry, list) {
+ if (entry->psi == psi) {
+ list_del_rcu(&entry->list);
+ psback->flag ^= 1 << entry->index;
+ synchronize_rcu();
+ free_buf_for_compression(entry->index);
+ kfree(entry);
+ break;
+ }
+ }
- psinfo = NULL;
- kfree(backend);
- backend = NULL;
+ if (psback->flag == PSOTRE_LIST_EMPTY) {
+ kfree(psback);
+ psback = NULL;
+ }
pr_info("Unregistered %s as persistent store backend\n", psi->name);
- mutex_unlock(&psinfo_lock);
+ mutex_unlock(&psback_lock);
}
EXPORT_SYMBOL_GPL(pstore_unregister);
-static void decompress_record(struct pstore_record *record)
+static void decompress_record(struct pstore_record *record, int pos)
{
int ret;
int unzipped_len;
@@ -702,13 +750,13 @@ static void decompress_record(struct pstore_record *record)
}
/* Missing compression buffer means compression was not initialized. */
- if (!big_oops_buf) {
+ if (!big_oops_buf[pos]) {
pr_warn("no decompression method initialized!\n");
return;
}
/* Allocate enough space to hold max decompression and ECC. */
- unzipped_len = big_oops_buf_sz;
+ unzipped_len = big_oops_buf_sz[pos];
workspace = kmalloc(unzipped_len + record->ecc_notice_size,
GFP_KERNEL);
if (!workspace)
@@ -748,7 +796,7 @@ static void decompress_record(struct pstore_record *record)
* error records.
*/
void pstore_get_backend_records(struct pstore_info *psi,
- struct dentry *root, int quiet)
+ struct dentry *root, int quiet, int pos)
{
int failed = 0;
unsigned int stop_loop = 65536;
@@ -784,7 +832,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
break;
}
- decompress_record(record);
+ decompress_record(record, pos);
rc = pstore_mkfile(root, record);
if (rc) {
/* pstore_mkfile() did not take record, so free it. */
@@ -810,7 +858,12 @@ void pstore_get_backend_records(struct pstore_info *psi,
static void pstore_dowork(struct work_struct *work)
{
- pstore_get_records(1);
+ struct pstore_info_list *entry;
+
+ mutex_lock(&psback_lock);
+ list_for_each_entry(entry, &psback->list_entry, list)
+ pstore_get_records(entry->psi, entry->index, 1);
+ mutex_unlock(&psback_lock);
}
static void pstore_timefunc(struct timer_list *unused)
@@ -841,6 +894,10 @@ static void __init pstore_choose_compression(void)
static int __init pstore_init(void)
{
int ret;
+ struct pstore_info_list *entry;
+
+ if (!psback)
+ return 0;
pstore_choose_compression();
@@ -849,11 +906,18 @@ static int __init pstore_init(void)
* initialize compression because crypto was not ready. If so,
* initialize compression now.
*/
- allocate_buf_for_compression();
+ mutex_lock(&psback_lock);
+ list_for_each_entry(entry, &psback->list_entry, list)
+ allocate_buf_for_compression(entry->psi, entry->index);
+ mutex_unlock(&psback_lock);
ret = pstore_init_fs();
- if (ret)
- free_buf_for_compression();
+ if (ret) {
+ mutex_lock(&psback_lock);
+ list_for_each_entry(entry, &psback->list_entry, list)
+ free_buf_for_compression(entry->index);
+ mutex_unlock(&psback_lock);
+ }
return ret;
}
diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c
index 55f139afa327..c35ce09861cf 100644
--- a/fs/pstore/pmsg.c
+++ b/fs/pstore/pmsg.c
@@ -11,8 +11,8 @@
static DEFINE_MUTEX(pmsg_lock);
-static ssize_t write_pmsg(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
+static ssize_t do_write_pmsg(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos, struct pstore_info *psinfo)
{
struct pstore_record record;
int ret;
@@ -34,6 +34,25 @@ static ssize_t write_pmsg(struct file *file, const char __user *buf,
return ret ? ret : count;
}
+static ssize_t write_pmsg(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int ret;
+ struct pstore_info_list *entry;
+
+ mutex_lock(&psback_lock);
+ list_for_each_entry(entry, &psback->list_entry, list) {
+ if (entry->psi->flags & PSTORE_FLAGS_PMSG) {
+ int _ret = do_write_pmsg(file, buf, count, ppos, entry->psi);
+
+ ret = ret > _ret ? ret : _ret;
+ }
+ }
+ mutex_unlock(&psback_lock);
+
+ return ret;
+}
+
static const struct file_operations pmsg_fops = {
.owner = THIS_MODULE,
.llseek = noop_llseek,
diff --git a/fs/pstore/tty.c b/fs/pstore/tty.c
index 432ed7194188..0cbe553bcdf2 100644
--- a/fs/pstore/tty.c
+++ b/fs/pstore/tty.c
@@ -41,7 +41,13 @@ void pstore_register_tty(void)
void pstore_start_tty(const unsigned char *buf, int count)
{
- do_write_ttymsg(buf, count);
+ struct pstore_info_list *entry;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_TTY)
+ do_write_ttymsg(buf, count, entry->psi);
+ rcu_read_unlock();
}
void pstore_unregister_tty(void)
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 791c86552921..ede60617d778 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -202,6 +202,33 @@ struct pstore_info {
int (*erase)(struct pstore_record *record);
};
+/* Supported multibackends */
+#define PSTORE_MAX_BACKEND_LENGTH 100
+#define PSTORE_BACKEND_NUM 16
+
+#define PSTORE_LIST_FULL (BIT(PSTORE_BACKEND_NUM) - 1)
+#define PSOTRE_LIST_EMPTY 0
+
+struct pstore_info_list {
+ struct pstore_info *psi;
+ struct list_head list;
+ int index;
+};
+
+/**
+ * struct pstore_backends - management of pstore backends
+ * @list_entry: entry of pstore backend driver information list
+ * @front_cnt: count of each enabled frontend
+ * @flag: bitmap of enabled pstore backend
+ *
+ */
+
+struct pstore_backends {
+ struct list_head list_entry;
+ int front_cnt[PSTORE_TYPE_MAX];
+ u16 flag;
+};
+
/* Supported frontends */
#define PSTORE_FLAGS_DMESG BIT(0)
#define PSTORE_FLAGS_CONSOLE BIT(1)
@@ -211,6 +238,8 @@ struct pstore_info {
extern int pstore_register(struct pstore_info *);
extern void pstore_unregister(struct pstore_info *);
+extern struct mutex psback_lock;
+
struct pstore_ftrace_record {
unsigned long ip;
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] pstore: add subdirs for multi-backends
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
2023-09-28 2:42 ` [PATCH 1/5] pstore: add tty frontend Yuanhe Shu
2023-09-28 2:42 ` [PATCH 2/5] pstore: add multi-backends support Yuanhe Shu
@ 2023-09-28 2:42 ` Yuanhe Shu
2023-09-28 2:42 ` [PATCH 4/5] pstore: remove the module parameter "backend" Yuanhe Shu
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Yuanhe Shu @ 2023-09-28 2:42 UTC (permalink / raw)
To: gregkh, jirislaby, keescook, tony.luck, gpiccoli, shuah
Cc: linux-kernel, linux-serial, linux-hardening, linux-kselftest,
Yuanhe Shu, Xingrui Yi
With pstore supporting multiple backends, since different backends
may enable different kinds of frontends, records in the file system
are very confusing.
By simply modifying the file system, subdirs are added for each backend,
each dir has the same name as the corresponding backend.
Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com>
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
fs/pstore/inode.c | 80 ++++++++++++++++++++++++++++++++++++------
fs/pstore/internal.h | 1 +
fs/pstore/platform.c | 5 ++-
include/linux/pstore.h | 2 ++
4 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 312e7d55b95f..0f6d888a3b06 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -288,9 +288,14 @@ static const struct super_operations pstore_ops = {
.show_options = pstore_show_options,
};
-static struct dentry *psinfo_lock_root(struct pstore_info *psinfo)
+static struct dentry *psinfo_lock(struct pstore_info *psinfo)
{
- struct dentry *root;
+ struct dentry *dentry;
+ struct qstr qname;
+
+ qname.name = psinfo->name;
+ qname.len = strlen(psinfo->name);
+ qname.hash = full_name_hash(pstore_sb->s_root, qname.name, qname.len);
mutex_lock(&pstore_sb_lock);
/*
@@ -302,11 +307,16 @@ static struct dentry *psinfo_lock_root(struct pstore_info *psinfo)
return NULL;
}
- root = pstore_sb->s_root;
- inode_lock(d_inode(root));
+ dentry = d_lookup(pstore_sb->s_root, &qname);
+ if (!dentry) {
+ mutex_unlock(&pstore_sb_lock);
+ return NULL;
+ }
+
+ inode_lock(d_inode(dentry));
mutex_unlock(&pstore_sb_lock);
- return root;
+ return dentry;
}
int pstore_put_backend_records(struct pstore_info *psi)
@@ -315,7 +325,7 @@ int pstore_put_backend_records(struct pstore_info *psi)
struct dentry *root;
int rc = 0;
- root = psinfo_lock_root(psi);
+ root = psinfo_lock(psi);
if (!root)
return 0;
@@ -416,14 +426,60 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
*/
void pstore_get_records(struct pstore_info *psi, int pos, int quiet)
{
+ struct dentry *dentry;
+
+ dentry = psinfo_lock(psi);
+ if (!dentry)
+ return;
+
+ pstore_get_backend_records(psi, dentry, quiet, pos);
+ inode_unlock(d_inode(dentry));
+}
+
+void pstore_mksubdir(struct pstore_info *psi)
+{
+ struct dentry *dentry;
+ struct inode *inode;
struct dentry *root;
+ struct qstr qname;
- root = psinfo_lock_root(psi);
- if (!root)
+ if (!psi || !pstore_sb)
+ return;
+
+ root = pstore_sb->s_root;
+ qname.name = psi->name;
+ qname.len = strlen(psi->name);
+ qname.hash = full_name_hash(root, qname.name, qname.len);
+ dentry = d_lookup(root, &qname);
+
+ /* Skip if subdir is already present in the filesystem. */
+ if (dentry)
return;
- pstore_get_backend_records(psi, root, quiet, pos);
+ inode_lock(d_inode(root));
+
+ dentry = d_alloc_name(root, psi->name);
+ if (!dentry)
+ goto fail;
+
+ inode = pstore_get_inode(pstore_sb);
+ if (!inode) {
+ dput(dentry);
+ dentry = ERR_PTR(-ENOMEM);
+ goto fail;
+ }
+ inode->i_mode = S_IFDIR | 0750;
+ inode->i_op = &pstore_dir_inode_operations;
+ inode->i_fop = &simple_dir_operations;
+
+ inc_nlink(inode);
+ inc_nlink(d_inode(root));
+
+ d_add(dentry, inode);
+
+fail:
inode_unlock(d_inode(root));
+ return;
}
static int pstore_fill_super(struct super_block *sb, void *data, int silent)
@@ -456,10 +512,14 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
mutex_unlock(&pstore_sb_lock);
rcu_read_lock();
- list_for_each_entry_rcu(entry, &psback->list_entry, list)
+ list_for_each_entry_rcu(entry, &psback->list_entry, list) {
+ pstore_mksubdir(entry->psi);
pstore_get_records(entry->psi, entry->index, 0);
+ }
rcu_read_unlock();
+ psback->fs_ready = true;
+
return 0;
}
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index 7465b5baf002..20449ca940bb 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -44,6 +44,7 @@ static inline void pstore_unregister_tty(void) {}
extern struct pstore_backends *psback;
extern void pstore_set_kmsg_bytes(int);
+extern void pstore_mksubdir(struct pstore_info *psi);
extern void pstore_get_records(struct pstore_info *psi, int pos,
int quiet);
extern void pstore_get_backend_records(struct pstore_info *psi,
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 34fb21e6ff4f..d576ee48527c 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -652,7 +652,10 @@ int pstore_register(struct pstore_info *psi)
if (psi->flags & PSTORE_FLAGS_DMESG && !psback->front_cnt[PSTORE_TYPE_DMESG])
allocate_buf_for_compression(psi, newpsi->index);
- pstore_get_records(psi, newpsi->index, 0);
+ if (psback->fs_ready) {
+ pstore_mksubdir(psi);
+ pstore_get_records(psi, newpsi->index, 0);
+ }
list_add_rcu(&newpsi->list, &psback->list_entry);
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index ede60617d778..306597545f2d 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -220,6 +220,7 @@ struct pstore_info_list {
* @list_entry: entry of pstore backend driver information list
* @front_cnt: count of each enabled frontend
* @flag: bitmap of enabled pstore backend
+ * @fs_ready: whether the pstore filesystem is ready
*
*/
@@ -227,6 +228,7 @@ struct pstore_backends {
struct list_head list_entry;
int front_cnt[PSTORE_TYPE_MAX];
u16 flag;
+ bool fs_ready;
};
/* Supported frontends */
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] pstore: remove the module parameter "backend"
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
` (2 preceding siblings ...)
2023-09-28 2:42 ` [PATCH 3/5] pstore: add subdirs for multi-backends Yuanhe Shu
@ 2023-09-28 2:42 ` Yuanhe Shu
2023-09-28 2:42 ` [PATCH 5/5] tools/pstore: update pstore selftests Yuanhe Shu
2023-09-29 3:49 ` [PATCH 0/5] pstore: add tty frontend and multi-backend Kees Cook
5 siblings, 0 replies; 10+ messages in thread
From: Yuanhe Shu @ 2023-09-28 2:42 UTC (permalink / raw)
To: gregkh, jirislaby, keescook, tony.luck, gpiccoli, shuah
Cc: linux-kernel, linux-serial, linux-hardening, linux-kselftest,
Yuanhe Shu, Xingrui Yi
Since all enabled backends have corresponding subdir in
/sys/fs/pstore, and the logic of finding unexpected is
changed by using psinfo_list rather than parameter backend,
it is not necessary to retain the parameter backend.
Remove the unnecessary module parameter "backend".
Signed-off-by: Xingrui Yi <yixingrui@linux.alibaba.com>
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
fs/pstore/platform.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index d576ee48527c..90c33c1ca39f 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -77,10 +77,6 @@ static DECLARE_WORK(pstore_work, pstore_dowork);
DEFINE_MUTEX(psback_lock);
struct pstore_backends *psback;
-static char *backend;
-module_param(backend, charp, 0444);
-MODULE_PARM_DESC(backend, "specific backend to use");
-
static char *compress =
#ifdef CONFIG_PSTORE_COMPRESS_DEFAULT
CONFIG_PSTORE_COMPRESS_DEFAULT;
@@ -675,12 +671,6 @@ int pstore_register(struct pstore_info *psi)
/* Start watching for new records, if desired. */
pstore_timer_kick();
- /*
- * Update the module parameter backend, so it is visible
- * through /sys/module/pstore/parameters/backend
- */
- backend = kstrdup(psi->name, GFP_KERNEL);
-
pr_info("Registered %s as persistent store backend\n", psi->name);
mutex_unlock(&psback_lock);
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] tools/pstore: update pstore selftests
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
` (3 preceding siblings ...)
2023-09-28 2:42 ` [PATCH 4/5] pstore: remove the module parameter "backend" Yuanhe Shu
@ 2023-09-28 2:42 ` Yuanhe Shu
2023-09-29 3:49 ` [PATCH 0/5] pstore: add tty frontend and multi-backend Kees Cook
5 siblings, 0 replies; 10+ messages in thread
From: Yuanhe Shu @ 2023-09-28 2:42 UTC (permalink / raw)
To: gregkh, jirislaby, keescook, tony.luck, gpiccoli, shuah
Cc: linux-kernel, linux-serial, linux-hardening, linux-kselftest,
Yuanhe Shu
Update pstore selftests as pstore now supports multi backends
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
tools/testing/selftests/pstore/common_tests | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/testing/selftests/pstore/common_tests b/tools/testing/selftests/pstore/common_tests
index 4509f0cc9c91..f50166f59ec4 100755
--- a/tools/testing/selftests/pstore/common_tests
+++ b/tools/testing/selftests/pstore/common_tests
@@ -73,10 +73,6 @@ rc=0
prlog "=== Pstore unit tests (`basename $0`) ==="
prlog "UUID="$UUID
-prlog -n "Checking pstore backend is registered ... "
-backend=`cat /sys/module/pstore/parameters/backend`
-show_result $?
-prlog -e "\tbackend=${backend}"
prlog -e "\tcmdline=`cat /proc/cmdline`"
if [ $rc -ne 0 ]; then
exit 1
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/5] pstore: add tty frontend and multi-backend
2023-09-28 2:42 [PATCH 0/5] pstore: add tty frontend and multi-backend Yuanhe Shu
` (4 preceding siblings ...)
2023-09-28 2:42 ` [PATCH 5/5] tools/pstore: update pstore selftests Yuanhe Shu
@ 2023-09-29 3:49 ` Kees Cook
2023-09-29 5:47 ` Greg KH
2023-11-24 21:43 ` Guilherme G. Piccoli
5 siblings, 2 replies; 10+ messages in thread
From: Kees Cook @ 2023-09-29 3:49 UTC (permalink / raw)
To: Yuanhe Shu
Cc: gregkh, jirislaby, tony.luck, gpiccoli, shuah, linux-kernel,
linux-serial, linux-hardening, linux-kselftest
On Thu, Sep 28, 2023 at 10:42:39AM +0800, Yuanhe Shu wrote:
> In public cloud scenario, if kdump service works abnormally,
> users cannot get vmcore. Without vmcore, user has no idea why the
> kernel crashed. Meanwhile, there is no additional information
> to find the reason why the kdump service is abnormal.
>
> One way is to obtain console messages through VNC. The drawback
> is that VNC is real-time, if user missed the timing to get the VNC
> output, the crash needs to be retriggered.
>
> Another way is to enable the console frontend of pstore and record the
> console messages to the pstore backend. On the one hand, the console
> logs only contain kernel printk logs and does not cover
> user-mode print logs. Although we can redirect user-mode logs to the
> pmsg frontend provided by pstore, user-mode information related to
> booting and kdump service vary from systemd, kdump.sh, and so on which
> makes redirection troublesome. So we added a tty frontend and save all
> logs of tty driver to the pstore backend.
This is a clever solution!
> Another problem is that currently pstore only supports a single backend.
> For debugging kdump problems, we hope to save the console logs and tty
> logs to the ramoops backend of pstore, as it will not be lost after
> rebooting. If the user has enabled another backend, the ramoops backend
> will not be registered. To this end, we add the multi-backend function
> to support simultaneous registration of multiple backends.
Ah very cool; I really like this idea. I'd wanted to do it for a while
just to make testing easier, but I hadn't had time to attempt it.
> Based on the above changes, we can enable pstore in the crashdump kernel
> and save the console logs and tty logs to the ramoops backend of pstore.
> After rebooting, we can view the relevant logs by mounting the pstore
> file system.
So, before I do a line-at-a-time review of this code, I'd like to
address some design issues first.
I really don't want to make behavioral differences when we don't have
to:
- The multi-backend will enable _all possible_ backends, and that's a
big change that will do weird things for some pstore users. I would
prefer a pstore option to opt-in to enabling all backends. Perhaps
have "pstore.backend=" be parsed with commas, so a list of backends
can be provided, or "all" for the "all backends" behavior.
- Moving the pstorefs files into a subdirectory will break userspace
immediately (e.g. systemd-pstore expects very specifically named
files). Using subdirectories seems like a good idea, but perhaps
we need hardlinks into the root pstorefs for the "first" backend,
or some other creative solution here.
Then some technical thoughts about the TTY frontend's behavior:
- That 2 pstore records are created for every line of TTY output
feels kind of inefficient, though I don't have a better idea.
This is really only doable as you have it because the ramoops
and zone backends treat the single prz as a circular buffer.
I wonder about supporting this on other backends like EFI, but
perhaps it's just not going to happen.
- I'd like to check with the TTY folks to see if this is the "right"
place to hook to get a copy of what's being written.
Thanks and let me know what you think!
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/5] pstore: add tty frontend and multi-backend
2023-09-29 3:49 ` [PATCH 0/5] pstore: add tty frontend and multi-backend Kees Cook
@ 2023-09-29 5:47 ` Greg KH
2023-11-24 21:43 ` Guilherme G. Piccoli
1 sibling, 0 replies; 10+ messages in thread
From: Greg KH @ 2023-09-29 5:47 UTC (permalink / raw)
To: Kees Cook
Cc: Yuanhe Shu, jirislaby, tony.luck, gpiccoli, shuah, linux-kernel,
linux-serial, linux-hardening, linux-kselftest
On Thu, Sep 28, 2023 at 08:49:02PM -0700, Kees Cook wrote:
> On Thu, Sep 28, 2023 at 10:42:39AM +0800, Yuanhe Shu wrote:
> - I'd like to check with the TTY folks to see if this is the "right"
> place to hook to get a copy of what's being written.
It depends on what you want to get. What exact data are you looking
for here? I couldn't figure it out and I think I already asked it in my
review of the "hook" location.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] pstore: add tty frontend and multi-backend
2023-09-29 3:49 ` [PATCH 0/5] pstore: add tty frontend and multi-backend Kees Cook
2023-09-29 5:47 ` Greg KH
@ 2023-11-24 21:43 ` Guilherme G. Piccoli
1 sibling, 0 replies; 10+ messages in thread
From: Guilherme G. Piccoli @ 2023-11-24 21:43 UTC (permalink / raw)
To: Kees Cook, Yuanhe Shu
Cc: gregkh, jirislaby, tony.luck, shuah, linux-kernel, linux-serial,
linux-hardening, linux-kselftest
Hi Yuanhe / Kees.
My apologies (and embarrassment) for responding almost 2mo later...
On 29/09/2023 00:49, Kees Cook wrote:
> [...]
>> Another problem is that currently pstore only supports a single backend.
>> For debugging kdump problems, we hope to save the console logs and tty
>> logs to the ramoops backend of pstore, as it will not be lost after
>> rebooting. If the user has enabled another backend, the ramoops backend
>> will not be registered. To this end, we add the multi-backend function
>> to support simultaneous registration of multiple backends.
>
> Ah very cool; I really like this idea. I'd wanted to do it for a while
> just to make testing easier, but I hadn't had time to attempt it.
I found the idea of multi-backend quite interesting, thanks for that!!!
And to add on what's Kees mentioned, not sure others' opinions but seems
to me this is a bit more straightforward / path-of-less-resistance than
the the tty frontend, so I'd suggest split the series and focus first on
this and once accepted, hook the tty thingy.
Not that the series can't be sent altogether, reviews could work in
parallel...I just see them as a bit tangential one to the other, personally.
> [...]
> - The multi-backend will enable _all possible_ backends, and that's a
> big change that will do weird things for some pstore users. I would
> prefer a pstore option to opt-in to enabling all backends. Perhaps
> have "pstore.backend=" be parsed with commas, so a list of backends
> can be provided, or "all" for the "all backends" behavior.
>
> - Moving the pstorefs files into a subdirectory will break userspace
> immediately (e.g. systemd-pstore expects very specifically named
> files). Using subdirectories seems like a good idea, but perhaps
> we need hardlinks into the root pstorefs for the "first" backend,
> or some other creative solution here.
>
Big +1 in these two, commas are a very nice idea and changing the sysfs
current way of exposing pstore logs would break at least kdumpst (the
Steam Deck/Arch pstore / kdump tool), besides systemd-pstore that was
already mentioned (and who knows what more tools / scripts out in the
field).
Overall, thanks a bunch for this work Yuanhe!
Cheers,
Guilherme
^ permalink raw reply [flat|nested] 10+ messages in thread