* [PATCH 0/1] ubifs: ubifs to export filesystem error counters
@ 2021-09-07 21:40 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-07 21:40 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel; +Cc: Stefan Schaeckeler
Not all ubifs filesystem errors are propagated to userspace, here is one
that is not propagaged to ls:
[node0_RP0_CPU0:~]$ls -la /mnt/mtd0
[562009.111309] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad CRC: calculated 0x1b205ba3, read 0xb6eff0d9
[562009.123231] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad node at LEB 112:29768
[562009.133042] magic 0x6101831
[562009.137312] crc 0xb6eff0d9
[562009.141678] node_type 2 (direntry node)
[562009.146706] group_type 1 (in node group)
[562009.151734] sqnum 334966
[562009.155709] len 82
[562009.159304] key (1, direntry, 0xe933a79)
[562009.164999] inum 546
[562009.168687] type 0
[562009.172186] nlen 25
[562009.175779] name inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
[562009.185308] CPU: 1 PID: 21670 Comm: ls Tainted: G O 4.8.28-WR9.0.0.20_cgl #1
[562009.185309] Hardware name: Insyde Harrisonville/Type2 - Board Product Name1, BIOS 00.01.017 10/26/2018
[562009.185312] 0000000000000286 00000000f877ad2e ffffbb5b8e357c50 ffffffff9b3cfaab
[562009.185316] 00000000ffffff8b 0000000000007448 ffffbb5b8e357c90 ffffffffc022c0dd
[562009.185320] 00000000687aa008 0000000000007448 0000000000000070 0000000000000052
[562009.185324] Call Trace:
[562009.185332] [<ffffffff9b3cfaab>] dump_stack+0x63/0x88
[562009.185345] [<ffffffffc022c0dd>] ubifs_check_node+0xbd/0x270 [ubifs]
[562009.185357] [<ffffffffc022db25>] ubifs_read_node+0x285/0x300 [ubifs]
[562009.185370] [<ffffffffc024dfc7>] ubifs_tnc_read_node+0x127/0x1c0 [ubifs]
[562009.185382] [<ffffffffc0230115>] ? matches_name+0x45/0xf0 [ubifs]
[562009.185394] [<ffffffffc022ed9a>] tnc_read_node_nm+0xfa/0x220 [ubifs]
[562009.185407] [<ffffffffc02330a4>] ubifs_tnc_next_ent+0x1f4/0x2a0 [ubifs]
[562009.185411] [<ffffffff9b1f405e>] ? filldir+0xce/0x150
[562009.185422] [<ffffffffc02244f8>] ubifs_readdir+0x188/0x4d0 [ubifs]
[562009.185425] [<ffffffff9b1f3e62>] iterate_dir+0x172/0x190
[562009.185429] [<ffffffff9b124e4a>] ? __audit_syscall_entry+0xba/0x100
[562009.185432] [<ffffffff9b1f4399>] SyS_getdents+0x99/0x120
[562009.185434] [<ffffffff9b1f3f90>] ? fillonedir+0x110/0x110
[562009.185437] [<ffffffff9b002b76>] do_syscall_64+0x66/0x180
[562009.185441] [<ffffffff9b8b9fce>] entry_SYSCALL_64_after_swapgs+0x58/0xc6
[562009.185454] UBIFS error (ubi0:0 pid 21670): ubifs_read_node [ubifs]: expected node type 2
[562009.194705] UBIFS error (ubi0:0 pid 21670): ubifs_readdir [ubifs]: cannot find next direntry, error -117
total 1080
drwxr-xr-x. 2 root root 3728 Jul 3 10:36 .
drwxrwxrwt. 5 root root 120 Jul 3 10:36 ..
-rw-rw-rw-. 1 root root 5413 May 24 10:59 alarm_0_PM1_remote_log_1.txt
-rw-rw-rw-. 1 root root 1 Sep 19 2020 alarm_banner.txt
-rw-rw-rw-. 1 root root 65465 Nov 5 2020 alarm_local_log_29.txt
-rw-rw-rw-. 1 root root 55441 Dec 18 2020 alarm_local_log_30.txt
-rw-rw-rw-. 1 root root 64826 Sep 20 2020 alarm_local_log_31.txt
-rw-rw-rw-. 1 root root 65019 Oct 25 2020 alarm_local_log_32.txt
-rw-rw-rw-. 1 root root 65880 Oct 26 2020 alarm_local_log_33.txt
-rw-rw-rw-. 1 root root 66296 Apr 17 10:35 alarm_local_log_34.txt
-rw-rw-rw-. 1 root test 64734 May 5 20:35 alarm_local_log_35.txt
-rw-rw-rw-. 1 root root 64924 Jun 4 07:39 alarm_local_log_36.txt
-rw-rw-rw-. 1 root root 50830 Jul 8 13:21 alarm_local_log_37.txt
-rw-rw-rw-. 1 root root 1 Sep 21 2020 fpd_banner.txt
-rw-rw-rw-. 1 root root 28518 Jul 2 23:19 fpd_local_log_2.txt
-rw-rw-rw-. 1 root root 96 Jul 9 22:37 int_uptime_dashboard.dat
-rw-rw-rw-. 1 root iosxr 540 Jun 8 00:06 inventory_local_log_1.txt
-rw-rw-rw-. 1 root iosxr 540 Jul 2 23:26 inventory_local_log_2.txt
-rw-rw-rw-. 1 root root 79293 Jul 3 10:37 inventory_local_log_3.txt
-rw-rw-rw-. 1 root test 4 Apr 17 10:33 obfl_data_version.dat
-rw-rw-rw-. 1 root root 364 Sep 19 2020 temperature_banner.txt
-rw-rw-rw-. 1 root root 19567 Dec 18 2020 temperature_local_log_3.txt
-rw-rw-rw-. 1 root root 65607 Sep 20 2020 temperature_local_log_4.txt
-rw-rw-rw-. 1 root root 66410 Apr 17 10:34 temperature_local_log_5.txt
-rw-rw-rw-. 1 root test 65607 Jun 1 21:26 temperature_local_log_6.txt
-rw-rw-rw-. 1 root root 54097 Jul 3 10:37 temperature_local_log_7.txt
-rw-rw-rw-. 1 root root 65160 Jun 25 23:56 voltage_local_log_10.txt
-rw-rw-rw-. 1 root root 28236 Jul 3 10:37 voltage_local_log_11.txt
[node0_RP0_CPU0:~]$echo $?
0
A direntry node got corrupted. The filename inventory_local_log_4.txt got
corrupted to inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
and is not passed down to ls. ls exits with a clean exit code of 0.
We can't really detect this corruption from user space. This is required to
take action such as for a fresh start with re-creating the filesystem.
Every access to the mounted filesystem results in a kernel backtrace. This
trashes the dmesg buffer and the systemd journal over time.
This patch introduces a sysfs filesystem for ubifs. The first three sysfs
nodes are error counters:
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
This allows userspace to notice filesystem corruption. Over time, more
sysfs nodes can be added.
Stefan Schaeckeler (1):
ubifs: ubifs to export filesystem error counters
fs/ubifs/Makefile | 2 +-
fs/ubifs/io.c | 6 ++
fs/ubifs/super.c | 17 ++++-
fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
fs/ubifs/sysfs.h | 39 ++++++++++
fs/ubifs/ubifs.h | 11 +++
6 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 fs/ubifs/sysfs.c
create mode 100644 fs/ubifs/sysfs.h
--
2.32.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/1] ubifs: ubifs to export filesystem error counters
@ 2021-09-07 21:40 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-07 21:40 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel; +Cc: Stefan Schaeckeler
Not all ubifs filesystem errors are propagated to userspace, here is one
that is not propagaged to ls:
[node0_RP0_CPU0:~]$ls -la /mnt/mtd0
[562009.111309] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad CRC: calculated 0x1b205ba3, read 0xb6eff0d9
[562009.123231] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad node at LEB 112:29768
[562009.133042] magic 0x6101831
[562009.137312] crc 0xb6eff0d9
[562009.141678] node_type 2 (direntry node)
[562009.146706] group_type 1 (in node group)
[562009.151734] sqnum 334966
[562009.155709] len 82
[562009.159304] key (1, direntry, 0xe933a79)
[562009.164999] inum 546
[562009.168687] type 0
[562009.172186] nlen 25
[562009.175779] name inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
[562009.185308] CPU: 1 PID: 21670 Comm: ls Tainted: G O 4.8.28-WR9.0.0.20_cgl #1
[562009.185309] Hardware name: Insyde Harrisonville/Type2 - Board Product Name1, BIOS 00.01.017 10/26/2018
[562009.185312] 0000000000000286 00000000f877ad2e ffffbb5b8e357c50 ffffffff9b3cfaab
[562009.185316] 00000000ffffff8b 0000000000007448 ffffbb5b8e357c90 ffffffffc022c0dd
[562009.185320] 00000000687aa008 0000000000007448 0000000000000070 0000000000000052
[562009.185324] Call Trace:
[562009.185332] [<ffffffff9b3cfaab>] dump_stack+0x63/0x88
[562009.185345] [<ffffffffc022c0dd>] ubifs_check_node+0xbd/0x270 [ubifs]
[562009.185357] [<ffffffffc022db25>] ubifs_read_node+0x285/0x300 [ubifs]
[562009.185370] [<ffffffffc024dfc7>] ubifs_tnc_read_node+0x127/0x1c0 [ubifs]
[562009.185382] [<ffffffffc0230115>] ? matches_name+0x45/0xf0 [ubifs]
[562009.185394] [<ffffffffc022ed9a>] tnc_read_node_nm+0xfa/0x220 [ubifs]
[562009.185407] [<ffffffffc02330a4>] ubifs_tnc_next_ent+0x1f4/0x2a0 [ubifs]
[562009.185411] [<ffffffff9b1f405e>] ? filldir+0xce/0x150
[562009.185422] [<ffffffffc02244f8>] ubifs_readdir+0x188/0x4d0 [ubifs]
[562009.185425] [<ffffffff9b1f3e62>] iterate_dir+0x172/0x190
[562009.185429] [<ffffffff9b124e4a>] ? __audit_syscall_entry+0xba/0x100
[562009.185432] [<ffffffff9b1f4399>] SyS_getdents+0x99/0x120
[562009.185434] [<ffffffff9b1f3f90>] ? fillonedir+0x110/0x110
[562009.185437] [<ffffffff9b002b76>] do_syscall_64+0x66/0x180
[562009.185441] [<ffffffff9b8b9fce>] entry_SYSCALL_64_after_swapgs+0x58/0xc6
[562009.185454] UBIFS error (ubi0:0 pid 21670): ubifs_read_node [ubifs]: expected node type 2
[562009.194705] UBIFS error (ubi0:0 pid 21670): ubifs_readdir [ubifs]: cannot find next direntry, error -117
total 1080
drwxr-xr-x. 2 root root 3728 Jul 3 10:36 .
drwxrwxrwt. 5 root root 120 Jul 3 10:36 ..
-rw-rw-rw-. 1 root root 5413 May 24 10:59 alarm_0_PM1_remote_log_1.txt
-rw-rw-rw-. 1 root root 1 Sep 19 2020 alarm_banner.txt
-rw-rw-rw-. 1 root root 65465 Nov 5 2020 alarm_local_log_29.txt
-rw-rw-rw-. 1 root root 55441 Dec 18 2020 alarm_local_log_30.txt
-rw-rw-rw-. 1 root root 64826 Sep 20 2020 alarm_local_log_31.txt
-rw-rw-rw-. 1 root root 65019 Oct 25 2020 alarm_local_log_32.txt
-rw-rw-rw-. 1 root root 65880 Oct 26 2020 alarm_local_log_33.txt
-rw-rw-rw-. 1 root root 66296 Apr 17 10:35 alarm_local_log_34.txt
-rw-rw-rw-. 1 root test 64734 May 5 20:35 alarm_local_log_35.txt
-rw-rw-rw-. 1 root root 64924 Jun 4 07:39 alarm_local_log_36.txt
-rw-rw-rw-. 1 root root 50830 Jul 8 13:21 alarm_local_log_37.txt
-rw-rw-rw-. 1 root root 1 Sep 21 2020 fpd_banner.txt
-rw-rw-rw-. 1 root root 28518 Jul 2 23:19 fpd_local_log_2.txt
-rw-rw-rw-. 1 root root 96 Jul 9 22:37 int_uptime_dashboard.dat
-rw-rw-rw-. 1 root iosxr 540 Jun 8 00:06 inventory_local_log_1.txt
-rw-rw-rw-. 1 root iosxr 540 Jul 2 23:26 inventory_local_log_2.txt
-rw-rw-rw-. 1 root root 79293 Jul 3 10:37 inventory_local_log_3.txt
-rw-rw-rw-. 1 root test 4 Apr 17 10:33 obfl_data_version.dat
-rw-rw-rw-. 1 root root 364 Sep 19 2020 temperature_banner.txt
-rw-rw-rw-. 1 root root 19567 Dec 18 2020 temperature_local_log_3.txt
-rw-rw-rw-. 1 root root 65607 Sep 20 2020 temperature_local_log_4.txt
-rw-rw-rw-. 1 root root 66410 Apr 17 10:34 temperature_local_log_5.txt
-rw-rw-rw-. 1 root test 65607 Jun 1 21:26 temperature_local_log_6.txt
-rw-rw-rw-. 1 root root 54097 Jul 3 10:37 temperature_local_log_7.txt
-rw-rw-rw-. 1 root root 65160 Jun 25 23:56 voltage_local_log_10.txt
-rw-rw-rw-. 1 root root 28236 Jul 3 10:37 voltage_local_log_11.txt
[node0_RP0_CPU0:~]$echo $?
0
A direntry node got corrupted. The filename inventory_local_log_4.txt got
corrupted to inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
and is not passed down to ls. ls exits with a clean exit code of 0.
We can't really detect this corruption from user space. This is required to
take action such as for a fresh start with re-creating the filesystem.
Every access to the mounted filesystem results in a kernel backtrace. This
trashes the dmesg buffer and the systemd journal over time.
This patch introduces a sysfs filesystem for ubifs. The first three sysfs
nodes are error counters:
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
This allows userspace to notice filesystem corruption. Over time, more
sysfs nodes can be added.
Stefan Schaeckeler (1):
ubifs: ubifs to export filesystem error counters
fs/ubifs/Makefile | 2 +-
fs/ubifs/io.c | 6 ++
fs/ubifs/super.c | 17 ++++-
fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
fs/ubifs/sysfs.h | 39 ++++++++++
fs/ubifs/ubifs.h | 11 +++
6 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 fs/ubifs/sysfs.c
create mode 100644 fs/ubifs/sysfs.h
--
2.32.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/1] ubifs: ubifs to export filesystem error counters
2021-09-07 21:40 ` Stefan Schaeckeler
@ 2021-09-07 21:40 ` Stefan Schaeckeler
-1 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-07 21:40 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel; +Cc: Stefan Schaeckeler, Stefan Schaeckeler
Not all ubifs filesystem errors are propagated to userspace.
Export bad magic, bad node and crc errors via sysfs. This allows userspace
to notice filesystem errors:
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
The counters are reset to 0 with a remount. Writing anything into the
counters also clears them.
Signed-off-by: Stefan Schaeckeler <sschaeck@cisco.com>
---
fs/ubifs/Makefile | 2 +-
fs/ubifs/io.c | 6 ++
fs/ubifs/super.c | 17 ++++-
fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
fs/ubifs/sysfs.h | 39 ++++++++++
fs/ubifs/ubifs.h | 11 +++
6 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 fs/ubifs/sysfs.c
create mode 100644 fs/ubifs/sysfs.h
diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
index 5c4b845754a7..314c80b24a76 100644
--- a/fs/ubifs/Makefile
+++ b/fs/ubifs/Makefile
@@ -5,7 +5,7 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o
ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
-ubifs-y += misc.o
+ubifs-y += misc.o sysfs.o
ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 00b61dba62b7..0b158e420cc1 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -238,6 +238,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (!quiet)
ubifs_err(c, "bad magic %#08x, expected %#08x",
magic, UBIFS_NODE_MAGIC);
+ if (c->stats)
+ c->stats->magic_errors++;
err = -EUCLEAN;
goto out;
}
@@ -246,6 +248,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (type < 0 || type >= UBIFS_NODE_TYPES_CNT) {
if (!quiet)
ubifs_err(c, "bad node type %d", type);
+ if (c->stats)
+ c->stats->node_errors++;
goto out;
}
@@ -270,6 +274,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (!quiet)
ubifs_err(c, "bad CRC: calculated %#08x, read %#08x",
crc, node_crc);
+ if (c->stats)
+ c->stats->crc_errors++;
err = -EUCLEAN;
goto out;
}
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index f0fb25727d96..50b934854a84 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -24,6 +24,7 @@
#include <linux/mount.h>
#include <linux/math64.h>
#include <linux/writeback.h>
+#include "sysfs.h"
#include "ubifs.h"
static int ubifs_default_version_set(const char *val, const struct kernel_param *kp)
@@ -1264,6 +1265,10 @@ static int mount_ubifs(struct ubifs_info *c)
if (err)
return err;
+ err = ubifs_sysfs_register(c);
+ if (err)
+ goto out_debugging;
+
err = check_volume_empty(c);
if (err)
goto out_free;
@@ -1641,6 +1646,8 @@ static int mount_ubifs(struct ubifs_info *c)
vfree(c->sbuf);
kfree(c->bottom_up_buf);
kfree(c->sup_node);
+ ubifs_sysfs_unregister(c);
+out_debugging:
ubifs_debugging_exit(c);
return err;
}
@@ -1684,6 +1691,7 @@ static void ubifs_umount(struct ubifs_info *c)
kfree(c->bottom_up_buf);
kfree(c->sup_node);
ubifs_debugging_exit(c);
+ ubifs_sysfs_unregister(c);
}
/**
@@ -2436,14 +2444,20 @@ static int __init ubifs_init(void)
dbg_debugfs_init();
+ err = ubifs_sysfs_init();
+ if (err)
+ goto out_dbg;
+
err = register_filesystem(&ubifs_fs_type);
if (err) {
pr_err("UBIFS error (pid %d): cannot register file system, error %d",
current->pid, err);
- goto out_dbg;
+ goto out_sysfs;
}
return 0;
+out_sysfs:
+ ubifs_sysfs_exit();
out_dbg:
dbg_debugfs_exit();
ubifs_compressors_exit();
@@ -2462,6 +2476,7 @@ static void __exit ubifs_exit(void)
WARN_ON(atomic_long_read(&ubifs_clean_zn_cnt) != 0);
dbg_debugfs_exit();
+ ubifs_sysfs_exit();
ubifs_compressors_exit();
unregister_shrinker(&ubifs_shrinker_info);
diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
new file mode 100644
index 000000000000..bac53a0f0451
--- /dev/null
+++ b/fs/ubifs/sysfs.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * This file is part of UBIFS.
+ *
+ * Copyright (C) 2021 Cisco Systems
+ *
+ * Author: Stefan Schaeckeler
+ */
+
+
+#include <linux/fs.h>
+
+#include "sysfs.h"
+#include "ubifs.h"
+
+
+enum attr_id_t {
+ attr_errors_magic,
+ attr_errors_node,
+ attr_errors_crc,
+};
+
+struct ubifs_attr {
+ struct attribute attr;
+ enum attr_id_t attr_id;
+};
+
+#define UBIFS_ATTR(_name, _mode, _id) \
+static struct ubifs_attr ubifs_attr_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr_id = attr_##_id, \
+}
+
+#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
+
+UBIFS_ATTR_FUNC(errors_magic, 0644);
+UBIFS_ATTR_FUNC(errors_crc, 0644);
+UBIFS_ATTR_FUNC(errors_node, 0644);
+
+#define ATTR_LIST(name) (&ubifs_attr_##name.attr)
+
+static struct attribute *ubifs_attrs[] = {
+ ATTR_LIST(errors_magic),
+ ATTR_LIST(errors_node),
+ ATTR_LIST(errors_crc),
+ NULL,
+};
+
+
+static ssize_t ubifs_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
+ kobj);
+
+ struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
+
+ switch (a->attr_id) {
+ case attr_errors_magic:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->magic_errors);
+ case attr_errors_node:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->node_errors);
+ case attr_errors_crc:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->crc_errors);
+ }
+ return 0;
+};
+
+
+static ssize_t ubifs_attr_store(struct kobject *kobj,
+ struct attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
+ kobj);
+
+ struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
+
+ switch (a->attr_id) {
+ case attr_errors_magic:
+ sbi->stats->magic_errors = 0;
+ break;
+ case attr_errors_node:
+ sbi->stats->node_errors = 0;
+ break;
+ case attr_errors_crc:
+ sbi->stats->crc_errors = 0;
+ break;
+ }
+ return len;
+}
+
+
+static void ubifs_sb_release(struct kobject *kobj)
+{
+ struct ubifs_info *c = container_of(kobj, struct ubifs_info, kobj);
+
+ complete(&c->kobj_unregister);
+}
+
+
+static const struct sysfs_ops ubifs_attr_ops = {
+ .show = ubifs_attr_show,
+ .store = ubifs_attr_store,
+};
+
+static struct kobj_type ubifs_sb_ktype = {
+ .default_attrs = ubifs_attrs,
+ .sysfs_ops = &ubifs_attr_ops,
+ .release = ubifs_sb_release,
+};
+
+static struct kobj_type ubifs_ktype = {
+ .sysfs_ops = &ubifs_attr_ops,
+};
+
+static struct kset ubifs_kset = {
+ .kobj = {.ktype = &ubifs_ktype},
+};
+
+
+int ubifs_sysfs_register(struct ubifs_info *c)
+{
+ int ret, n;
+ char dfs_dir_name[UBIFS_DFS_DIR_LEN+1];
+
+ c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL);
+ if (!c->stats) {
+ ret = -ENOMEM;
+ goto out_last;
+ }
+ n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME,
+ c->vi.ubi_num, c->vi.vol_id);
+
+ if (n == UBIFS_DFS_DIR_LEN) {
+ /* The array size is too small */
+ ret = -EINVAL;
+ goto out_last;
+ }
+
+ c->kobj.kset = &ubifs_kset;
+ init_completion(&c->kobj_unregister);
+
+
+ ret = kobject_init_and_add(&c->kobj, &ubifs_sb_ktype, NULL,
+ "%s", dfs_dir_name);
+ if (ret)
+ goto out_put;
+
+ return 0;
+
+out_put:
+ kobject_put(&c->kobj);
+ wait_for_completion(&c->kobj_unregister);
+out_last:
+ ubifs_err(c, "cannot create sysfs entry for ubifs%d_%d, error %d\n",
+ c->vi.ubi_num, c->vi.vol_id, ret);
+ return ret;
+}
+
+void ubifs_sysfs_unregister(struct ubifs_info *c)
+{
+ kobject_del(&c->kobj);
+ kobject_put(&c->kobj);
+ wait_for_completion(&c->kobj_unregister);
+
+ kfree(c->stats);
+}
+
+int __init ubifs_sysfs_init(void)
+{
+ int ret;
+
+ kobject_set_name(&ubifs_kset.kobj, "ubifs");
+ ubifs_kset.kobj.parent = fs_kobj;
+ ret = kset_register(&ubifs_kset);
+
+ return ret;
+}
+
+void ubifs_sysfs_exit(void)
+{
+ kset_unregister(&ubifs_kset);
+}
diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
new file mode 100644
index 000000000000..a10a82585af8
--- /dev/null
+++ b/fs/ubifs/sysfs.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * This file is part of UBIFS.
+ *
+ * Copyright (C) 2021 Cisco Systems
+ *
+ * Author: Stefan Schaeckeler
+ */
+
+#ifndef __UBIFS_SYSFS_H__
+#define __UBIFS_SYSFS_H__
+
+struct ubifs_info;
+
+/*
+ * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi"
+ * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte.
+ */
+#define UBIFS_DFS_DIR_NAME "ubi%d_%d"
+#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1)
+
+/**
+ * ubifs_stats_info - per-FS statistics information.
+ * @magic_errors: number of bad magic numbers (will be reset with a new mount).
+ * @node_errors: number of bad nodes (will be reset with a new mount).
+ * @crc_errors: number of bad crcs (will be reset with a new mount).
+ */
+struct ubifs_stats_info {
+ unsigned int magic_errors;
+ unsigned int node_errors;
+ unsigned int crc_errors;
+};
+
+int ubifs_sysfs_init(void);
+void ubifs_sysfs_exit(void);
+int ubifs_sysfs_register(struct ubifs_info *c);
+void ubifs_sysfs_unregister(struct ubifs_info *c);
+
+#endif
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index c38066ce9ab0..bfc0f20b41a1 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -27,12 +27,15 @@
#include <linux/security.h>
#include <linux/xattr.h>
#include <linux/random.h>
+#include <linux/sysfs.h>
+#include <linux/completion.h>
#include <crypto/hash_info.h>
#include <crypto/hash.h>
#include <crypto/algapi.h>
#include <linux/fscrypt.h>
+#include "sysfs.h"
#include "ubifs-media.h"
/* Version of this UBIFS implementation */
@@ -1251,6 +1254,10 @@ struct ubifs_debug_info;
* @mount_opts: UBIFS-specific mount options
*
* @dbg: debugging-related information
+ * @stats: statistics exported over sysfs
+ *
+ * @kobj: kobject for /sys/fs/ubifs/
+ * @kobj_unregister: completion to unregister sysfs kobject
*/
struct ubifs_info {
struct super_block *vfs_sb;
@@ -1286,6 +1293,9 @@ struct ubifs_info {
spinlock_t cs_lock;
wait_queue_head_t cmt_wq;
+ struct kobject kobj;
+ struct completion kobj_unregister;
+
unsigned int big_lpt:1;
unsigned int space_fixup:1;
unsigned int double_hash:1;
@@ -1493,6 +1503,7 @@ struct ubifs_info {
struct ubifs_mount_opts mount_opts;
struct ubifs_debug_info *dbg;
+ struct ubifs_stats_info *stats;
};
extern struct list_head ubifs_infos;
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/1] ubifs: ubifs to export filesystem error counters
@ 2021-09-07 21:40 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-07 21:40 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel; +Cc: Stefan Schaeckeler, Stefan Schaeckeler
Not all ubifs filesystem errors are propagated to userspace.
Export bad magic, bad node and crc errors via sysfs. This allows userspace
to notice filesystem errors:
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
The counters are reset to 0 with a remount. Writing anything into the
counters also clears them.
Signed-off-by: Stefan Schaeckeler <sschaeck@cisco.com>
---
fs/ubifs/Makefile | 2 +-
fs/ubifs/io.c | 6 ++
fs/ubifs/super.c | 17 ++++-
fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
fs/ubifs/sysfs.h | 39 ++++++++++
fs/ubifs/ubifs.h | 11 +++
6 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 fs/ubifs/sysfs.c
create mode 100644 fs/ubifs/sysfs.h
diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
index 5c4b845754a7..314c80b24a76 100644
--- a/fs/ubifs/Makefile
+++ b/fs/ubifs/Makefile
@@ -5,7 +5,7 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o
ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
-ubifs-y += misc.o
+ubifs-y += misc.o sysfs.o
ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 00b61dba62b7..0b158e420cc1 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -238,6 +238,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (!quiet)
ubifs_err(c, "bad magic %#08x, expected %#08x",
magic, UBIFS_NODE_MAGIC);
+ if (c->stats)
+ c->stats->magic_errors++;
err = -EUCLEAN;
goto out;
}
@@ -246,6 +248,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (type < 0 || type >= UBIFS_NODE_TYPES_CNT) {
if (!quiet)
ubifs_err(c, "bad node type %d", type);
+ if (c->stats)
+ c->stats->node_errors++;
goto out;
}
@@ -270,6 +274,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void *buf, int len,
if (!quiet)
ubifs_err(c, "bad CRC: calculated %#08x, read %#08x",
crc, node_crc);
+ if (c->stats)
+ c->stats->crc_errors++;
err = -EUCLEAN;
goto out;
}
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index f0fb25727d96..50b934854a84 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -24,6 +24,7 @@
#include <linux/mount.h>
#include <linux/math64.h>
#include <linux/writeback.h>
+#include "sysfs.h"
#include "ubifs.h"
static int ubifs_default_version_set(const char *val, const struct kernel_param *kp)
@@ -1264,6 +1265,10 @@ static int mount_ubifs(struct ubifs_info *c)
if (err)
return err;
+ err = ubifs_sysfs_register(c);
+ if (err)
+ goto out_debugging;
+
err = check_volume_empty(c);
if (err)
goto out_free;
@@ -1641,6 +1646,8 @@ static int mount_ubifs(struct ubifs_info *c)
vfree(c->sbuf);
kfree(c->bottom_up_buf);
kfree(c->sup_node);
+ ubifs_sysfs_unregister(c);
+out_debugging:
ubifs_debugging_exit(c);
return err;
}
@@ -1684,6 +1691,7 @@ static void ubifs_umount(struct ubifs_info *c)
kfree(c->bottom_up_buf);
kfree(c->sup_node);
ubifs_debugging_exit(c);
+ ubifs_sysfs_unregister(c);
}
/**
@@ -2436,14 +2444,20 @@ static int __init ubifs_init(void)
dbg_debugfs_init();
+ err = ubifs_sysfs_init();
+ if (err)
+ goto out_dbg;
+
err = register_filesystem(&ubifs_fs_type);
if (err) {
pr_err("UBIFS error (pid %d): cannot register file system, error %d",
current->pid, err);
- goto out_dbg;
+ goto out_sysfs;
}
return 0;
+out_sysfs:
+ ubifs_sysfs_exit();
out_dbg:
dbg_debugfs_exit();
ubifs_compressors_exit();
@@ -2462,6 +2476,7 @@ static void __exit ubifs_exit(void)
WARN_ON(atomic_long_read(&ubifs_clean_zn_cnt) != 0);
dbg_debugfs_exit();
+ ubifs_sysfs_exit();
ubifs_compressors_exit();
unregister_shrinker(&ubifs_shrinker_info);
diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
new file mode 100644
index 000000000000..bac53a0f0451
--- /dev/null
+++ b/fs/ubifs/sysfs.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * This file is part of UBIFS.
+ *
+ * Copyright (C) 2021 Cisco Systems
+ *
+ * Author: Stefan Schaeckeler
+ */
+
+
+#include <linux/fs.h>
+
+#include "sysfs.h"
+#include "ubifs.h"
+
+
+enum attr_id_t {
+ attr_errors_magic,
+ attr_errors_node,
+ attr_errors_crc,
+};
+
+struct ubifs_attr {
+ struct attribute attr;
+ enum attr_id_t attr_id;
+};
+
+#define UBIFS_ATTR(_name, _mode, _id) \
+static struct ubifs_attr ubifs_attr_##_name = { \
+ .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr_id = attr_##_id, \
+}
+
+#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
+
+UBIFS_ATTR_FUNC(errors_magic, 0644);
+UBIFS_ATTR_FUNC(errors_crc, 0644);
+UBIFS_ATTR_FUNC(errors_node, 0644);
+
+#define ATTR_LIST(name) (&ubifs_attr_##name.attr)
+
+static struct attribute *ubifs_attrs[] = {
+ ATTR_LIST(errors_magic),
+ ATTR_LIST(errors_node),
+ ATTR_LIST(errors_crc),
+ NULL,
+};
+
+
+static ssize_t ubifs_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
+ kobj);
+
+ struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
+
+ switch (a->attr_id) {
+ case attr_errors_magic:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->magic_errors);
+ case attr_errors_node:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->node_errors);
+ case attr_errors_crc:
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ sbi->stats->crc_errors);
+ }
+ return 0;
+};
+
+
+static ssize_t ubifs_attr_store(struct kobject *kobj,
+ struct attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
+ kobj);
+
+ struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
+
+ switch (a->attr_id) {
+ case attr_errors_magic:
+ sbi->stats->magic_errors = 0;
+ break;
+ case attr_errors_node:
+ sbi->stats->node_errors = 0;
+ break;
+ case attr_errors_crc:
+ sbi->stats->crc_errors = 0;
+ break;
+ }
+ return len;
+}
+
+
+static void ubifs_sb_release(struct kobject *kobj)
+{
+ struct ubifs_info *c = container_of(kobj, struct ubifs_info, kobj);
+
+ complete(&c->kobj_unregister);
+}
+
+
+static const struct sysfs_ops ubifs_attr_ops = {
+ .show = ubifs_attr_show,
+ .store = ubifs_attr_store,
+};
+
+static struct kobj_type ubifs_sb_ktype = {
+ .default_attrs = ubifs_attrs,
+ .sysfs_ops = &ubifs_attr_ops,
+ .release = ubifs_sb_release,
+};
+
+static struct kobj_type ubifs_ktype = {
+ .sysfs_ops = &ubifs_attr_ops,
+};
+
+static struct kset ubifs_kset = {
+ .kobj = {.ktype = &ubifs_ktype},
+};
+
+
+int ubifs_sysfs_register(struct ubifs_info *c)
+{
+ int ret, n;
+ char dfs_dir_name[UBIFS_DFS_DIR_LEN+1];
+
+ c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL);
+ if (!c->stats) {
+ ret = -ENOMEM;
+ goto out_last;
+ }
+ n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME,
+ c->vi.ubi_num, c->vi.vol_id);
+
+ if (n == UBIFS_DFS_DIR_LEN) {
+ /* The array size is too small */
+ ret = -EINVAL;
+ goto out_last;
+ }
+
+ c->kobj.kset = &ubifs_kset;
+ init_completion(&c->kobj_unregister);
+
+
+ ret = kobject_init_and_add(&c->kobj, &ubifs_sb_ktype, NULL,
+ "%s", dfs_dir_name);
+ if (ret)
+ goto out_put;
+
+ return 0;
+
+out_put:
+ kobject_put(&c->kobj);
+ wait_for_completion(&c->kobj_unregister);
+out_last:
+ ubifs_err(c, "cannot create sysfs entry for ubifs%d_%d, error %d\n",
+ c->vi.ubi_num, c->vi.vol_id, ret);
+ return ret;
+}
+
+void ubifs_sysfs_unregister(struct ubifs_info *c)
+{
+ kobject_del(&c->kobj);
+ kobject_put(&c->kobj);
+ wait_for_completion(&c->kobj_unregister);
+
+ kfree(c->stats);
+}
+
+int __init ubifs_sysfs_init(void)
+{
+ int ret;
+
+ kobject_set_name(&ubifs_kset.kobj, "ubifs");
+ ubifs_kset.kobj.parent = fs_kobj;
+ ret = kset_register(&ubifs_kset);
+
+ return ret;
+}
+
+void ubifs_sysfs_exit(void)
+{
+ kset_unregister(&ubifs_kset);
+}
diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
new file mode 100644
index 000000000000..a10a82585af8
--- /dev/null
+++ b/fs/ubifs/sysfs.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * This file is part of UBIFS.
+ *
+ * Copyright (C) 2021 Cisco Systems
+ *
+ * Author: Stefan Schaeckeler
+ */
+
+#ifndef __UBIFS_SYSFS_H__
+#define __UBIFS_SYSFS_H__
+
+struct ubifs_info;
+
+/*
+ * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi"
+ * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte.
+ */
+#define UBIFS_DFS_DIR_NAME "ubi%d_%d"
+#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1)
+
+/**
+ * ubifs_stats_info - per-FS statistics information.
+ * @magic_errors: number of bad magic numbers (will be reset with a new mount).
+ * @node_errors: number of bad nodes (will be reset with a new mount).
+ * @crc_errors: number of bad crcs (will be reset with a new mount).
+ */
+struct ubifs_stats_info {
+ unsigned int magic_errors;
+ unsigned int node_errors;
+ unsigned int crc_errors;
+};
+
+int ubifs_sysfs_init(void);
+void ubifs_sysfs_exit(void);
+int ubifs_sysfs_register(struct ubifs_info *c);
+void ubifs_sysfs_unregister(struct ubifs_info *c);
+
+#endif
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index c38066ce9ab0..bfc0f20b41a1 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -27,12 +27,15 @@
#include <linux/security.h>
#include <linux/xattr.h>
#include <linux/random.h>
+#include <linux/sysfs.h>
+#include <linux/completion.h>
#include <crypto/hash_info.h>
#include <crypto/hash.h>
#include <crypto/algapi.h>
#include <linux/fscrypt.h>
+#include "sysfs.h"
#include "ubifs-media.h"
/* Version of this UBIFS implementation */
@@ -1251,6 +1254,10 @@ struct ubifs_debug_info;
* @mount_opts: UBIFS-specific mount options
*
* @dbg: debugging-related information
+ * @stats: statistics exported over sysfs
+ *
+ * @kobj: kobject for /sys/fs/ubifs/
+ * @kobj_unregister: completion to unregister sysfs kobject
*/
struct ubifs_info {
struct super_block *vfs_sb;
@@ -1286,6 +1293,9 @@ struct ubifs_info {
spinlock_t cs_lock;
wait_queue_head_t cmt_wq;
+ struct kobject kobj;
+ struct completion kobj_unregister;
+
unsigned int big_lpt:1;
unsigned int space_fixup:1;
unsigned int double_hash:1;
@@ -1493,6 +1503,7 @@ struct ubifs_info {
struct ubifs_mount_opts mount_opts;
struct ubifs_debug_info *dbg;
+ struct ubifs_stats_info *stats;
};
extern struct list_head ubifs_infos;
--
2.32.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
2021-09-07 21:40 ` Stefan Schaeckeler
@ 2021-09-20 2:48 ` Stefan Schaeckeler
-1 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-20 2:48 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel
Hello mtd/ubifs,
I just want to check back on what you think about having a sysfs for ubifs and
starting with these three callbacks
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
Stefan
> From: Stefan Schaeckeler <schaecsn@gmx.net>
>
> Not all ubifs filesystem errors are propagated to userspace, here is one
> that is not propagaged to ls:
>
> [node0_RP0_CPU0:~]$ls -la /mnt/mtd0
> [562009.111309] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad CRC: calculated 0x1b205ba3, read 0xb6eff0d9
> [562009.123231] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad node at LEB 112:29768
> [562009.133042] magic 0x6101831
> [562009.137312] crc 0xb6eff0d9
> [562009.141678] node_type 2 (direntry node)
> [562009.146706] group_type 1 (in node group)
> [562009.151734] sqnum 334966
> [562009.155709] len 82
> [562009.159304] key (1, direntry, 0xe933a79)
> [562009.164999] inum 546
> [562009.168687] type 0
> [562009.172186] nlen 25
> [562009.175779] name inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
> [562009.185308] CPU: 1 PID: 21670 Comm: ls Tainted: G O 4.8.28-WR9.0.0.20_cgl #1
> [562009.185309] Hardware name: Insyde Harrisonville/Type2 - Board Product Name1, BIOS 00.01.017 10/26/2018
> [562009.185312] 0000000000000286 00000000f877ad2e ffffbb5b8e357c50 ffffffff9b3cfaab
> [562009.185316] 00000000ffffff8b 0000000000007448 ffffbb5b8e357c90 ffffffffc022c0dd
> [562009.185320] 00000000687aa008 0000000000007448 0000000000000070 0000000000000052
> [562009.185324] Call Trace:
> [562009.185332] [<ffffffff9b3cfaab>] dump_stack+0x63/0x88
> [562009.185345] [<ffffffffc022c0dd>] ubifs_check_node+0xbd/0x270 [ubifs]
> [562009.185357] [<ffffffffc022db25>] ubifs_read_node+0x285/0x300 [ubifs]
> [562009.185370] [<ffffffffc024dfc7>] ubifs_tnc_read_node+0x127/0x1c0 [ubifs]
> [562009.185382] [<ffffffffc0230115>] ? matches_name+0x45/0xf0 [ubifs]
> [562009.185394] [<ffffffffc022ed9a>] tnc_read_node_nm+0xfa/0x220 [ubifs]
> [562009.185407] [<ffffffffc02330a4>] ubifs_tnc_next_ent+0x1f4/0x2a0 [ubifs]
> [562009.185411] [<ffffffff9b1f405e>] ? filldir+0xce/0x150
> [562009.185422] [<ffffffffc02244f8>] ubifs_readdir+0x188/0x4d0 [ubifs]
> [562009.185425] [<ffffffff9b1f3e62>] iterate_dir+0x172/0x190
> [562009.185429] [<ffffffff9b124e4a>] ? __audit_syscall_entry+0xba/0x100
> [562009.185432] [<ffffffff9b1f4399>] SyS_getdents+0x99/0x120
> [562009.185434] [<ffffffff9b1f3f90>] ? fillonedir+0x110/0x110
> [562009.185437] [<ffffffff9b002b76>] do_syscall_64+0x66/0x180
> [562009.185441] [<ffffffff9b8b9fce>] entry_SYSCALL_64_after_swapgs+0x58/0xc6
> [562009.185454] UBIFS error (ubi0:0 pid 21670): ubifs_read_node [ubifs]: expected node type 2
> [562009.194705] UBIFS error (ubi0:0 pid 21670): ubifs_readdir [ubifs]: cannot find next direntry, error -117
> total 1080
> drwxr-xr-x. 2 root root 3728 Jul 3 10:36 .
> drwxrwxrwt. 5 root root 120 Jul 3 10:36 ..
> -rw-rw-rw-. 1 root root 5413 May 24 10:59 alarm_0_PM1_remote_log_1.txt
> -rw-rw-rw-. 1 root root 1 Sep 19 2020 alarm_banner.txt
> -rw-rw-rw-. 1 root root 65465 Nov 5 2020 alarm_local_log_29.txt
> -rw-rw-rw-. 1 root root 55441 Dec 18 2020 alarm_local_log_30.txt
> -rw-rw-rw-. 1 root root 64826 Sep 20 2020 alarm_local_log_31.txt
> -rw-rw-rw-. 1 root root 65019 Oct 25 2020 alarm_local_log_32.txt
> -rw-rw-rw-. 1 root root 65880 Oct 26 2020 alarm_local_log_33.txt
> -rw-rw-rw-. 1 root root 66296 Apr 17 10:35 alarm_local_log_34.txt
> -rw-rw-rw-. 1 root test 64734 May 5 20:35 alarm_local_log_35.txt
> -rw-rw-rw-. 1 root root 64924 Jun 4 07:39 alarm_local_log_36.txt
> -rw-rw-rw-. 1 root root 50830 Jul 8 13:21 alarm_local_log_37.txt
> -rw-rw-rw-. 1 root root 1 Sep 21 2020 fpd_banner.txt
> -rw-rw-rw-. 1 root root 28518 Jul 2 23:19 fpd_local_log_2.txt
> -rw-rw-rw-. 1 root root 96 Jul 9 22:37 int_uptime_dashboard.dat
> -rw-rw-rw-. 1 root iosxr 540 Jun 8 00:06 inventory_local_log_1.txt
> -rw-rw-rw-. 1 root iosxr 540 Jul 2 23:26 inventory_local_log_2.txt
> -rw-rw-rw-. 1 root root 79293 Jul 3 10:37 inventory_local_log_3.txt
> -rw-rw-rw-. 1 root test 4 Apr 17 10:33 obfl_data_version.dat
> -rw-rw-rw-. 1 root root 364 Sep 19 2020 temperature_banner.txt
> -rw-rw-rw-. 1 root root 19567 Dec 18 2020 temperature_local_log_3.txt
> -rw-rw-rw-. 1 root root 65607 Sep 20 2020 temperature_local_log_4.txt
> -rw-rw-rw-. 1 root root 66410 Apr 17 10:34 temperature_local_log_5.txt
> -rw-rw-rw-. 1 root test 65607 Jun 1 21:26 temperature_local_log_6.txt
> -rw-rw-rw-. 1 root root 54097 Jul 3 10:37 temperature_local_log_7.txt
> -rw-rw-rw-. 1 root root 65160 Jun 25 23:56 voltage_local_log_10.txt
> -rw-rw-rw-. 1 root root 28236 Jul 3 10:37 voltage_local_log_11.txt
> [node0_RP0_CPU0:~]$echo $?
> 0
>
> A direntry node got corrupted. The filename inventory_local_log_4.txt got
> corrupted to inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
> and is not passed down to ls. ls exits with a clean exit code of 0.
>
> We can't really detect this corruption from user space. This is required to
> take action such as for a fresh start with re-creating the filesystem.
>
> Every access to the mounted filesystem results in a kernel backtrace. This
> trashes the dmesg buffer and the systemd journal over time.
>
>
> This patch introduces a sysfs filesystem for ubifs. The first three sysfs
> nodes are error counters:
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
>
> This allows userspace to notice filesystem corruption. Over time, more
> sysfs nodes can be added.
>
> Stefan Schaeckeler (1):
> ubifs: ubifs to export filesystem error counters
>
> fs/ubifs/Makefile | 2 +-
> fs/ubifs/io.c | 6 ++
> fs/ubifs/super.c | 17 ++++-
> fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/ubifs/sysfs.h | 39 ++++++++++
> fs/ubifs/ubifs.h | 11 +++
> 6 files changed, 260 insertions(+), 2 deletions(-)
> create mode 100644 fs/ubifs/sysfs.c
> create mode 100644 fs/ubifs/sysfs.h
>
> --
> 2.32.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
@ 2021-09-20 2:48 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-09-20 2:48 UTC (permalink / raw)
To: richard, linux-mtd, linux-kernel
Hello mtd/ubifs,
I just want to check back on what you think about having a sysfs for ubifs and
starting with these three callbacks
/sys/fs/ubifs/ubiX_Y/errors_magic
/sys/fs/ubifs/ubiX_Y/errors_node
/sys/fs/ubifs/ubiX_Y/errors_crc
Stefan
> From: Stefan Schaeckeler <schaecsn@gmx.net>
>
> Not all ubifs filesystem errors are propagated to userspace, here is one
> that is not propagaged to ls:
>
> [node0_RP0_CPU0:~]$ls -la /mnt/mtd0
> [562009.111309] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad CRC: calculated 0x1b205ba3, read 0xb6eff0d9
> [562009.123231] UBIFS error (ubi0:0 pid 21670): ubifs_check_node [ubifs]: bad node at LEB 112:29768
> [562009.133042] magic 0x6101831
> [562009.137312] crc 0xb6eff0d9
> [562009.141678] node_type 2 (direntry node)
> [562009.146706] group_type 1 (in node group)
> [562009.151734] sqnum 334966
> [562009.155709] len 82
> [562009.159304] key (1, direntry, 0xe933a79)
> [562009.164999] inum 546
> [562009.168687] type 0
> [562009.172186] nlen 25
> [562009.175779] name inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
> [562009.185308] CPU: 1 PID: 21670 Comm: ls Tainted: G O 4.8.28-WR9.0.0.20_cgl #1
> [562009.185309] Hardware name: Insyde Harrisonville/Type2 - Board Product Name1, BIOS 00.01.017 10/26/2018
> [562009.185312] 0000000000000286 00000000f877ad2e ffffbb5b8e357c50 ffffffff9b3cfaab
> [562009.185316] 00000000ffffff8b 0000000000007448 ffffbb5b8e357c90 ffffffffc022c0dd
> [562009.185320] 00000000687aa008 0000000000007448 0000000000000070 0000000000000052
> [562009.185324] Call Trace:
> [562009.185332] [<ffffffff9b3cfaab>] dump_stack+0x63/0x88
> [562009.185345] [<ffffffffc022c0dd>] ubifs_check_node+0xbd/0x270 [ubifs]
> [562009.185357] [<ffffffffc022db25>] ubifs_read_node+0x285/0x300 [ubifs]
> [562009.185370] [<ffffffffc024dfc7>] ubifs_tnc_read_node+0x127/0x1c0 [ubifs]
> [562009.185382] [<ffffffffc0230115>] ? matches_name+0x45/0xf0 [ubifs]
> [562009.185394] [<ffffffffc022ed9a>] tnc_read_node_nm+0xfa/0x220 [ubifs]
> [562009.185407] [<ffffffffc02330a4>] ubifs_tnc_next_ent+0x1f4/0x2a0 [ubifs]
> [562009.185411] [<ffffffff9b1f405e>] ? filldir+0xce/0x150
> [562009.185422] [<ffffffffc02244f8>] ubifs_readdir+0x188/0x4d0 [ubifs]
> [562009.185425] [<ffffffff9b1f3e62>] iterate_dir+0x172/0x190
> [562009.185429] [<ffffffff9b124e4a>] ? __audit_syscall_entry+0xba/0x100
> [562009.185432] [<ffffffff9b1f4399>] SyS_getdents+0x99/0x120
> [562009.185434] [<ffffffff9b1f3f90>] ? fillonedir+0x110/0x110
> [562009.185437] [<ffffffff9b002b76>] do_syscall_64+0x66/0x180
> [562009.185441] [<ffffffff9b8b9fce>] entry_SYSCALL_64_after_swapgs+0x58/0xc6
> [562009.185454] UBIFS error (ubi0:0 pid 21670): ubifs_read_node [ubifs]: expected node type 2
> [562009.194705] UBIFS error (ubi0:0 pid 21670): ubifs_readdir [ubifs]: cannot find next direntry, error -117
> total 1080
> drwxr-xr-x. 2 root root 3728 Jul 3 10:36 .
> drwxrwxrwt. 5 root root 120 Jul 3 10:36 ..
> -rw-rw-rw-. 1 root root 5413 May 24 10:59 alarm_0_PM1_remote_log_1.txt
> -rw-rw-rw-. 1 root root 1 Sep 19 2020 alarm_banner.txt
> -rw-rw-rw-. 1 root root 65465 Nov 5 2020 alarm_local_log_29.txt
> -rw-rw-rw-. 1 root root 55441 Dec 18 2020 alarm_local_log_30.txt
> -rw-rw-rw-. 1 root root 64826 Sep 20 2020 alarm_local_log_31.txt
> -rw-rw-rw-. 1 root root 65019 Oct 25 2020 alarm_local_log_32.txt
> -rw-rw-rw-. 1 root root 65880 Oct 26 2020 alarm_local_log_33.txt
> -rw-rw-rw-. 1 root root 66296 Apr 17 10:35 alarm_local_log_34.txt
> -rw-rw-rw-. 1 root test 64734 May 5 20:35 alarm_local_log_35.txt
> -rw-rw-rw-. 1 root root 64924 Jun 4 07:39 alarm_local_log_36.txt
> -rw-rw-rw-. 1 root root 50830 Jul 8 13:21 alarm_local_log_37.txt
> -rw-rw-rw-. 1 root root 1 Sep 21 2020 fpd_banner.txt
> -rw-rw-rw-. 1 root root 28518 Jul 2 23:19 fpd_local_log_2.txt
> -rw-rw-rw-. 1 root root 96 Jul 9 22:37 int_uptime_dashboard.dat
> -rw-rw-rw-. 1 root iosxr 540 Jun 8 00:06 inventory_local_log_1.txt
> -rw-rw-rw-. 1 root iosxr 540 Jul 2 23:26 inventory_local_log_2.txt
> -rw-rw-rw-. 1 root root 79293 Jul 3 10:37 inventory_local_log_3.txt
> -rw-rw-rw-. 1 root test 4 Apr 17 10:33 obfl_data_version.dat
> -rw-rw-rw-. 1 root root 364 Sep 19 2020 temperature_banner.txt
> -rw-rw-rw-. 1 root root 19567 Dec 18 2020 temperature_local_log_3.txt
> -rw-rw-rw-. 1 root root 65607 Sep 20 2020 temperature_local_log_4.txt
> -rw-rw-rw-. 1 root root 66410 Apr 17 10:34 temperature_local_log_5.txt
> -rw-rw-rw-. 1 root test 65607 Jun 1 21:26 temperature_local_log_6.txt
> -rw-rw-rw-. 1 root root 54097 Jul 3 10:37 temperature_local_log_7.txt
> -rw-rw-rw-. 1 root root 65160 Jun 25 23:56 voltage_local_log_10.txt
> -rw-rw-rw-. 1 root root 28236 Jul 3 10:37 voltage_local_log_11.txt
> [node0_RP0_CPU0:~]$echo $?
> 0
>
> A direntry node got corrupted. The filename inventory_local_log_4.txt got
> corrupted to inventor\xffffffff\xffffffff\xffffffff\xffffffffcal_log_4.txt
> and is not passed down to ls. ls exits with a clean exit code of 0.
>
> We can't really detect this corruption from user space. This is required to
> take action such as for a fresh start with re-creating the filesystem.
>
> Every access to the mounted filesystem results in a kernel backtrace. This
> trashes the dmesg buffer and the systemd journal over time.
>
>
> This patch introduces a sysfs filesystem for ubifs. The first three sysfs
> nodes are error counters:
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
>
> This allows userspace to notice filesystem corruption. Over time, more
> sysfs nodes can be added.
>
> Stefan Schaeckeler (1):
> ubifs: ubifs to export filesystem error counters
>
> fs/ubifs/Makefile | 2 +-
> fs/ubifs/io.c | 6 ++
> fs/ubifs/super.c | 17 ++++-
> fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/ubifs/sysfs.h | 39 ++++++++++
> fs/ubifs/ubifs.h | 11 +++
> 6 files changed, 260 insertions(+), 2 deletions(-)
> create mode 100644 fs/ubifs/sysfs.c
> create mode 100644 fs/ubifs/sysfs.h
>
> --
> 2.32.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
2021-09-20 2:48 ` Stefan Schaeckeler
@ 2021-09-20 21:57 ` Richard Weinberger
-1 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-09-20 21:57 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel
Hi!
----- Ursprüngliche Mail -----
> Von: "Stefan Schaeckeler" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> Gesendet: Montag, 20. September 2021 04:48:24
> Betreff: Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
> Hello mtd/ubifs,
>
> I just want to check back on what you think about having a sysfs for ubifs and
> starting with these three callbacks
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
I didn't have a chance to review your changes to far but in general I like the idea
if exposing such an interface.
Sorry for being a slow maintainer.
Thanks,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
@ 2021-09-20 21:57 ` Richard Weinberger
0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-09-20 21:57 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel
Hi!
----- Ursprüngliche Mail -----
> Von: "Stefan Schaeckeler" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> Gesendet: Montag, 20. September 2021 04:48:24
> Betreff: Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
> Hello mtd/ubifs,
>
> I just want to check back on what you think about having a sysfs for ubifs and
> starting with these three callbacks
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
I didn't have a chance to review your changes to far but in general I like the idea
if exposing such an interface.
Sorry for being a slow maintainer.
Thanks,
//richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
2021-09-20 21:57 ` Richard Weinberger
@ 2021-10-02 21:12 ` Stefan Schaeckeler
-1 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-10-02 21:12 UTC (permalink / raw)
To: richard; +Cc: linux-mtd, linux-kernel
Hello Richard,
> > Hello mtd/ubifs,
> >
> > I just want to check back on what you think about having a sysfs for ubifs and
> > starting with these three callbacks
> >
> > /sys/fs/ubifs/ubiX_Y/errors_magic
> > /sys/fs/ubifs/ubiX_Y/errors_node
> > /sys/fs/ubifs/ubiX_Y/errors_crc
>
> I didn't have a chance to review your changes to far but in general I like the idea
> if exposing such an interface.
> Sorry for being a slow maintainer.
I just wonder if you still like the idea of a sysfs for ubifs. At one point we
should then move forward.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/1] ubifs: ubifs to export filesystem error counters
@ 2021-10-02 21:12 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-10-02 21:12 UTC (permalink / raw)
To: richard; +Cc: linux-mtd, linux-kernel
Hello Richard,
> > Hello mtd/ubifs,
> >
> > I just want to check back on what you think about having a sysfs for ubifs and
> > starting with these three callbacks
> >
> > /sys/fs/ubifs/ubiX_Y/errors_magic
> > /sys/fs/ubifs/ubiX_Y/errors_node
> > /sys/fs/ubifs/ubiX_Y/errors_crc
>
> I didn't have a chance to review your changes to far but in general I like the idea
> if exposing such an interface.
> Sorry for being a slow maintainer.
I just wonder if you still like the idea of a sysfs for ubifs. At one point we
should then move forward.
Stefan
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
2021-09-07 21:40 ` Stefan Schaeckeler
@ 2021-10-03 19:58 ` Richard Weinberger
-1 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-10-03 19:58 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel, Stefan Schaeckeler
Stefan,
----- Ursprüngliche Mail -----
> Von: "schaecsn" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> CC: "schaecsn" <schaecsn@gmx.net>, "Stefan Schaeckeler" <sschaeck@cisco.com>
> Gesendet: Dienstag, 7. September 2021 23:40:34
> Betreff: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
> Not all ubifs filesystem errors are propagated to userspace.
>
> Export bad magic, bad node and crc errors via sysfs. This allows userspace
> to notice filesystem errors:
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
>
> The counters are reset to 0 with a remount. Writing anything into the
> counters also clears them.
I think this is a nice feature. Thanks for implementing it.
Please see some minor nits below.
Is there a specific reason why you didn't implement it via UBIFS's debugfs interface?
sysfs is ABI, so we cannot change much anymore.
> Signed-off-by: Stefan Schaeckeler <sschaeck@cisco.com>
> ---
> fs/ubifs/Makefile | 2 +-
> fs/ubifs/io.c | 6 ++
> fs/ubifs/super.c | 17 ++++-
> fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/ubifs/sysfs.h | 39 ++++++++++
> fs/ubifs/ubifs.h | 11 +++
> 6 files changed, 260 insertions(+), 2 deletions(-)
> create mode 100644 fs/ubifs/sysfs.c
> create mode 100644 fs/ubifs/sysfs.h
>
> diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
> index 5c4b845754a7..314c80b24a76 100644
> --- a/fs/ubifs/Makefile
> +++ b/fs/ubifs/Makefile
> @@ -5,7 +5,7 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o
> ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
> ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
> ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
> -ubifs-y += misc.o
> +ubifs-y += misc.o sysfs.o
> ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
> ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
> ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 00b61dba62b7..0b158e420cc1 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -238,6 +238,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (!quiet)
> ubifs_err(c, "bad magic %#08x, expected %#08x",
> magic, UBIFS_NODE_MAGIC);
> + if (c->stats)
> + c->stats->magic_errors++;
Please wrap this into a helper function.
> err = -EUCLEAN;
> goto out;
> }
> @@ -246,6 +248,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (type < 0 || type >= UBIFS_NODE_TYPES_CNT) {
> if (!quiet)
> ubifs_err(c, "bad node type %d", type);
> + if (c->stats)
> + c->stats->node_errors++;
Same.
> goto out;
> }
>
> @@ -270,6 +274,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (!quiet)
> ubifs_err(c, "bad CRC: calculated %#08x, read %#08x",
> crc, node_crc);
> + if (c->stats)
> + c->stats->crc_errors++;
Same.
> err = -EUCLEAN;
> goto out;
> }
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index f0fb25727d96..50b934854a84 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -24,6 +24,7 @@
> #include <linux/mount.h>
> #include <linux/math64.h>
> #include <linux/writeback.h>
> +#include "sysfs.h"
> #include "ubifs.h"
>
> static int ubifs_default_version_set(const char *val, const struct kernel_param
> *kp)
> @@ -1264,6 +1265,10 @@ static int mount_ubifs(struct ubifs_info *c)
> if (err)
> return err;
>
> + err = ubifs_sysfs_register(c);
> + if (err)
> + goto out_debugging;
> +
> err = check_volume_empty(c);
> if (err)
> goto out_free;
> @@ -1641,6 +1646,8 @@ static int mount_ubifs(struct ubifs_info *c)
> vfree(c->sbuf);
> kfree(c->bottom_up_buf);
> kfree(c->sup_node);
> + ubifs_sysfs_unregister(c);
> +out_debugging:
> ubifs_debugging_exit(c);
> return err;
> }
> @@ -1684,6 +1691,7 @@ static void ubifs_umount(struct ubifs_info *c)
> kfree(c->bottom_up_buf);
> kfree(c->sup_node);
> ubifs_debugging_exit(c);
> + ubifs_sysfs_unregister(c);
> }
>
> /**
> @@ -2436,14 +2444,20 @@ static int __init ubifs_init(void)
>
> dbg_debugfs_init();
>
> + err = ubifs_sysfs_init();
> + if (err)
> + goto out_dbg;
> +
> err = register_filesystem(&ubifs_fs_type);
> if (err) {
> pr_err("UBIFS error (pid %d): cannot register file system, error %d",
> current->pid, err);
> - goto out_dbg;
> + goto out_sysfs;
> }
> return 0;
>
> +out_sysfs:
> + ubifs_sysfs_exit();
> out_dbg:
> dbg_debugfs_exit();
> ubifs_compressors_exit();
> @@ -2462,6 +2476,7 @@ static void __exit ubifs_exit(void)
> WARN_ON(atomic_long_read(&ubifs_clean_zn_cnt) != 0);
>
> dbg_debugfs_exit();
> + ubifs_sysfs_exit();
> ubifs_compressors_exit();
> unregister_shrinker(&ubifs_shrinker_info);
>
> diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
> new file mode 100644
> index 000000000000..bac53a0f0451
> --- /dev/null
> +++ b/fs/ubifs/sysfs.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * This file is part of UBIFS.
> + *
> + * Copyright (C) 2021 Cisco Systems
> + *
> + * Author: Stefan Schaeckeler
> + */
> +
> +
> +#include <linux/fs.h>
> +
> +#include "sysfs.h"
> +#include "ubifs.h"
> +
> +
> +enum attr_id_t {
> + attr_errors_magic,
> + attr_errors_node,
> + attr_errors_crc,
> +};
> +
> +struct ubifs_attr {
> + struct attribute attr;
> + enum attr_id_t attr_id;
> +};
> +
> +#define UBIFS_ATTR(_name, _mode, _id) \
> +static struct ubifs_attr ubifs_attr_##_name = { \
> + .attr = {.name = __stringify(_name), .mode = _mode }, \
> + .attr_id = attr_##_id, \
> +}
> +
> +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
> +
> +UBIFS_ATTR_FUNC(errors_magic, 0644);
> +UBIFS_ATTR_FUNC(errors_crc, 0644);
> +UBIFS_ATTR_FUNC(errors_node, 0644);
I'm not sure whether everyone should be allowed to read these stats.
dmesg is also restricted these days. An unprivileged user should not see the
errors he can indirectly trigger.
> +#define ATTR_LIST(name) (&ubifs_attr_##name.attr)
> +
> +static struct attribute *ubifs_attrs[] = {
> + ATTR_LIST(errors_magic),
> + ATTR_LIST(errors_node),
> + ATTR_LIST(errors_crc),
> + NULL,
> +};
> +
> +
> +static ssize_t ubifs_attr_show(struct kobject *kobj,
> + struct attribute *attr, char *buf)
> +{
> + struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
> + kobj);
> +
> + struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
> +
> + switch (a->attr_id) {
> + case attr_errors_magic:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->magic_errors);
> + case attr_errors_node:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->node_errors);
> + case attr_errors_crc:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->crc_errors);
Please use sysfs_emit().
> + }
> + return 0;
> +};
> +
> +
> +static ssize_t ubifs_attr_store(struct kobject *kobj,
> + struct attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
> + kobj);
> +
> + struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
> +
> + switch (a->attr_id) {
> + case attr_errors_magic:
> + sbi->stats->magic_errors = 0;
> + break;
> + case attr_errors_node:
> + sbi->stats->node_errors = 0;
> + break;
> + case attr_errors_crc:
> + sbi->stats->crc_errors = 0;
> + break;
> + }
> + return len;
> +}
> +
> +
> +static void ubifs_sb_release(struct kobject *kobj)
> +{
> + struct ubifs_info *c = container_of(kobj, struct ubifs_info, kobj);
> +
> + complete(&c->kobj_unregister);
> +}
> +
> +
> +static const struct sysfs_ops ubifs_attr_ops = {
> + .show = ubifs_attr_show,
> + .store = ubifs_attr_store,
> +};
> +
> +static struct kobj_type ubifs_sb_ktype = {
> + .default_attrs = ubifs_attrs,
> + .sysfs_ops = &ubifs_attr_ops,
> + .release = ubifs_sb_release,
> +};
> +
> +static struct kobj_type ubifs_ktype = {
> + .sysfs_ops = &ubifs_attr_ops,
> +};
> +
> +static struct kset ubifs_kset = {
> + .kobj = {.ktype = &ubifs_ktype},
> +};
> +
> +
> +int ubifs_sysfs_register(struct ubifs_info *c)
> +{
> + int ret, n;
> + char dfs_dir_name[UBIFS_DFS_DIR_LEN+1];
> +
> + c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL);
> + if (!c->stats) {
> + ret = -ENOMEM;
> + goto out_last;
> + }
> + n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME,
> + c->vi.ubi_num, c->vi.vol_id);
> +
> + if (n == UBIFS_DFS_DIR_LEN) {
> + /* The array size is too small */
> + ret = -EINVAL;
> + goto out_last;
Where is c->stats released in case of an error?
> + }
> +
> + c->kobj.kset = &ubifs_kset;
> + init_completion(&c->kobj_unregister);
> +
> +
> + ret = kobject_init_and_add(&c->kobj, &ubifs_sb_ktype, NULL,
> + "%s", dfs_dir_name);
> + if (ret)
> + goto out_put;
> +
> + return 0;
> +
> +out_put:
> + kobject_put(&c->kobj);
> + wait_for_completion(&c->kobj_unregister);
> +out_last:
> + ubifs_err(c, "cannot create sysfs entry for ubifs%d_%d, error %d\n",
> + c->vi.ubi_num, c->vi.vol_id, ret);
> + return ret;
> +}
> +
> +void ubifs_sysfs_unregister(struct ubifs_info *c)
> +{
> + kobject_del(&c->kobj);
> + kobject_put(&c->kobj);
> + wait_for_completion(&c->kobj_unregister);
> +
> + kfree(c->stats);
> +}
> +
> +int __init ubifs_sysfs_init(void)
> +{
> + int ret;
> +
> + kobject_set_name(&ubifs_kset.kobj, "ubifs");
> + ubifs_kset.kobj.parent = fs_kobj;
> + ret = kset_register(&ubifs_kset);
> +
> + return ret;
> +}
> +
> +void ubifs_sysfs_exit(void)
> +{
> + kset_unregister(&ubifs_kset);
> +}
> diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
> new file mode 100644
> index 000000000000..a10a82585af8
> --- /dev/null
> +++ b/fs/ubifs/sysfs.h
Do we really need a new header file?
Usually most run-time stuff of UBIFS is part of ubifs.h.
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * This file is part of UBIFS.
> + *
> + * Copyright (C) 2021 Cisco Systems
> + *
> + * Author: Stefan Schaeckeler
> + */
> +
> +#ifndef __UBIFS_SYSFS_H__
> +#define __UBIFS_SYSFS_H__
> +
> +struct ubifs_info;
> +
> +/*
> + * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi"
> + * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte.
> + */
> +#define UBIFS_DFS_DIR_NAME "ubi%d_%d"
> +#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1)
> +
> +/**
> + * ubifs_stats_info - per-FS statistics information.
> + * @magic_errors: number of bad magic numbers (will be reset with a new mount).
> + * @node_errors: number of bad nodes (will be reset with a new mount).
> + * @crc_errors: number of bad crcs (will be reset with a new mount).
> + */
> +struct ubifs_stats_info {
> + unsigned int magic_errors;
> + unsigned int node_errors;
> + unsigned int crc_errors;
> +};
> +
> +int ubifs_sysfs_init(void);
> +void ubifs_sysfs_exit(void);
> +int ubifs_sysfs_register(struct ubifs_info *c);
> +void ubifs_sysfs_unregister(struct ubifs_info *c);
> +
> +#endif
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index c38066ce9ab0..bfc0f20b41a1 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -27,12 +27,15 @@
> #include <linux/security.h>
> #include <linux/xattr.h>
> #include <linux/random.h>
> +#include <linux/sysfs.h>
> +#include <linux/completion.h>
> #include <crypto/hash_info.h>
> #include <crypto/hash.h>
> #include <crypto/algapi.h>
>
> #include <linux/fscrypt.h>
>
> +#include "sysfs.h"
> #include "ubifs-media.h"
>
> /* Version of this UBIFS implementation */
> @@ -1251,6 +1254,10 @@ struct ubifs_debug_info;
> * @mount_opts: UBIFS-specific mount options
> *
> * @dbg: debugging-related information
> + * @stats: statistics exported over sysfs
> + *
> + * @kobj: kobject for /sys/fs/ubifs/
> + * @kobj_unregister: completion to unregister sysfs kobject
> */
> struct ubifs_info {
> struct super_block *vfs_sb;
> @@ -1286,6 +1293,9 @@ struct ubifs_info {
> spinlock_t cs_lock;
> wait_queue_head_t cmt_wq;
>
> + struct kobject kobj;
> + struct completion kobj_unregister;
> +
> unsigned int big_lpt:1;
> unsigned int space_fixup:1;
> unsigned int double_hash:1;
> @@ -1493,6 +1503,7 @@ struct ubifs_info {
> struct ubifs_mount_opts mount_opts;
>
> struct ubifs_debug_info *dbg;
> + struct ubifs_stats_info *stats;
> };
>
> extern struct list_head ubifs_infos;
> --
> 2.32.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
@ 2021-10-03 19:58 ` Richard Weinberger
0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-10-03 19:58 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel, Stefan Schaeckeler
Stefan,
----- Ursprüngliche Mail -----
> Von: "schaecsn" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> CC: "schaecsn" <schaecsn@gmx.net>, "Stefan Schaeckeler" <sschaeck@cisco.com>
> Gesendet: Dienstag, 7. September 2021 23:40:34
> Betreff: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
> Not all ubifs filesystem errors are propagated to userspace.
>
> Export bad magic, bad node and crc errors via sysfs. This allows userspace
> to notice filesystem errors:
>
> /sys/fs/ubifs/ubiX_Y/errors_magic
> /sys/fs/ubifs/ubiX_Y/errors_node
> /sys/fs/ubifs/ubiX_Y/errors_crc
>
> The counters are reset to 0 with a remount. Writing anything into the
> counters also clears them.
I think this is a nice feature. Thanks for implementing it.
Please see some minor nits below.
Is there a specific reason why you didn't implement it via UBIFS's debugfs interface?
sysfs is ABI, so we cannot change much anymore.
> Signed-off-by: Stefan Schaeckeler <sschaeck@cisco.com>
> ---
> fs/ubifs/Makefile | 2 +-
> fs/ubifs/io.c | 6 ++
> fs/ubifs/super.c | 17 ++++-
> fs/ubifs/sysfs.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/ubifs/sysfs.h | 39 ++++++++++
> fs/ubifs/ubifs.h | 11 +++
> 6 files changed, 260 insertions(+), 2 deletions(-)
> create mode 100644 fs/ubifs/sysfs.c
> create mode 100644 fs/ubifs/sysfs.h
>
> diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
> index 5c4b845754a7..314c80b24a76 100644
> --- a/fs/ubifs/Makefile
> +++ b/fs/ubifs/Makefile
> @@ -5,7 +5,7 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o
> ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
> ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
> ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
> -ubifs-y += misc.o
> +ubifs-y += misc.o sysfs.o
> ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
> ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
> ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
> index 00b61dba62b7..0b158e420cc1 100644
> --- a/fs/ubifs/io.c
> +++ b/fs/ubifs/io.c
> @@ -238,6 +238,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (!quiet)
> ubifs_err(c, "bad magic %#08x, expected %#08x",
> magic, UBIFS_NODE_MAGIC);
> + if (c->stats)
> + c->stats->magic_errors++;
Please wrap this into a helper function.
> err = -EUCLEAN;
> goto out;
> }
> @@ -246,6 +248,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (type < 0 || type >= UBIFS_NODE_TYPES_CNT) {
> if (!quiet)
> ubifs_err(c, "bad node type %d", type);
> + if (c->stats)
> + c->stats->node_errors++;
Same.
> goto out;
> }
>
> @@ -270,6 +274,8 @@ int ubifs_check_node(const struct ubifs_info *c, const void
> *buf, int len,
> if (!quiet)
> ubifs_err(c, "bad CRC: calculated %#08x, read %#08x",
> crc, node_crc);
> + if (c->stats)
> + c->stats->crc_errors++;
Same.
> err = -EUCLEAN;
> goto out;
> }
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index f0fb25727d96..50b934854a84 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -24,6 +24,7 @@
> #include <linux/mount.h>
> #include <linux/math64.h>
> #include <linux/writeback.h>
> +#include "sysfs.h"
> #include "ubifs.h"
>
> static int ubifs_default_version_set(const char *val, const struct kernel_param
> *kp)
> @@ -1264,6 +1265,10 @@ static int mount_ubifs(struct ubifs_info *c)
> if (err)
> return err;
>
> + err = ubifs_sysfs_register(c);
> + if (err)
> + goto out_debugging;
> +
> err = check_volume_empty(c);
> if (err)
> goto out_free;
> @@ -1641,6 +1646,8 @@ static int mount_ubifs(struct ubifs_info *c)
> vfree(c->sbuf);
> kfree(c->bottom_up_buf);
> kfree(c->sup_node);
> + ubifs_sysfs_unregister(c);
> +out_debugging:
> ubifs_debugging_exit(c);
> return err;
> }
> @@ -1684,6 +1691,7 @@ static void ubifs_umount(struct ubifs_info *c)
> kfree(c->bottom_up_buf);
> kfree(c->sup_node);
> ubifs_debugging_exit(c);
> + ubifs_sysfs_unregister(c);
> }
>
> /**
> @@ -2436,14 +2444,20 @@ static int __init ubifs_init(void)
>
> dbg_debugfs_init();
>
> + err = ubifs_sysfs_init();
> + if (err)
> + goto out_dbg;
> +
> err = register_filesystem(&ubifs_fs_type);
> if (err) {
> pr_err("UBIFS error (pid %d): cannot register file system, error %d",
> current->pid, err);
> - goto out_dbg;
> + goto out_sysfs;
> }
> return 0;
>
> +out_sysfs:
> + ubifs_sysfs_exit();
> out_dbg:
> dbg_debugfs_exit();
> ubifs_compressors_exit();
> @@ -2462,6 +2476,7 @@ static void __exit ubifs_exit(void)
> WARN_ON(atomic_long_read(&ubifs_clean_zn_cnt) != 0);
>
> dbg_debugfs_exit();
> + ubifs_sysfs_exit();
> ubifs_compressors_exit();
> unregister_shrinker(&ubifs_shrinker_info);
>
> diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
> new file mode 100644
> index 000000000000..bac53a0f0451
> --- /dev/null
> +++ b/fs/ubifs/sysfs.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * This file is part of UBIFS.
> + *
> + * Copyright (C) 2021 Cisco Systems
> + *
> + * Author: Stefan Schaeckeler
> + */
> +
> +
> +#include <linux/fs.h>
> +
> +#include "sysfs.h"
> +#include "ubifs.h"
> +
> +
> +enum attr_id_t {
> + attr_errors_magic,
> + attr_errors_node,
> + attr_errors_crc,
> +};
> +
> +struct ubifs_attr {
> + struct attribute attr;
> + enum attr_id_t attr_id;
> +};
> +
> +#define UBIFS_ATTR(_name, _mode, _id) \
> +static struct ubifs_attr ubifs_attr_##_name = { \
> + .attr = {.name = __stringify(_name), .mode = _mode }, \
> + .attr_id = attr_##_id, \
> +}
> +
> +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
> +
> +UBIFS_ATTR_FUNC(errors_magic, 0644);
> +UBIFS_ATTR_FUNC(errors_crc, 0644);
> +UBIFS_ATTR_FUNC(errors_node, 0644);
I'm not sure whether everyone should be allowed to read these stats.
dmesg is also restricted these days. An unprivileged user should not see the
errors he can indirectly trigger.
> +#define ATTR_LIST(name) (&ubifs_attr_##name.attr)
> +
> +static struct attribute *ubifs_attrs[] = {
> + ATTR_LIST(errors_magic),
> + ATTR_LIST(errors_node),
> + ATTR_LIST(errors_crc),
> + NULL,
> +};
> +
> +
> +static ssize_t ubifs_attr_show(struct kobject *kobj,
> + struct attribute *attr, char *buf)
> +{
> + struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
> + kobj);
> +
> + struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
> +
> + switch (a->attr_id) {
> + case attr_errors_magic:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->magic_errors);
> + case attr_errors_node:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->node_errors);
> + case attr_errors_crc:
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + sbi->stats->crc_errors);
Please use sysfs_emit().
> + }
> + return 0;
> +};
> +
> +
> +static ssize_t ubifs_attr_store(struct kobject *kobj,
> + struct attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct ubifs_info *sbi = container_of(kobj, struct ubifs_info,
> + kobj);
> +
> + struct ubifs_attr *a = container_of(attr, struct ubifs_attr, attr);
> +
> + switch (a->attr_id) {
> + case attr_errors_magic:
> + sbi->stats->magic_errors = 0;
> + break;
> + case attr_errors_node:
> + sbi->stats->node_errors = 0;
> + break;
> + case attr_errors_crc:
> + sbi->stats->crc_errors = 0;
> + break;
> + }
> + return len;
> +}
> +
> +
> +static void ubifs_sb_release(struct kobject *kobj)
> +{
> + struct ubifs_info *c = container_of(kobj, struct ubifs_info, kobj);
> +
> + complete(&c->kobj_unregister);
> +}
> +
> +
> +static const struct sysfs_ops ubifs_attr_ops = {
> + .show = ubifs_attr_show,
> + .store = ubifs_attr_store,
> +};
> +
> +static struct kobj_type ubifs_sb_ktype = {
> + .default_attrs = ubifs_attrs,
> + .sysfs_ops = &ubifs_attr_ops,
> + .release = ubifs_sb_release,
> +};
> +
> +static struct kobj_type ubifs_ktype = {
> + .sysfs_ops = &ubifs_attr_ops,
> +};
> +
> +static struct kset ubifs_kset = {
> + .kobj = {.ktype = &ubifs_ktype},
> +};
> +
> +
> +int ubifs_sysfs_register(struct ubifs_info *c)
> +{
> + int ret, n;
> + char dfs_dir_name[UBIFS_DFS_DIR_LEN+1];
> +
> + c->stats = kzalloc(sizeof(struct ubifs_stats_info), GFP_KERNEL);
> + if (!c->stats) {
> + ret = -ENOMEM;
> + goto out_last;
> + }
> + n = snprintf(dfs_dir_name, UBIFS_DFS_DIR_LEN + 1, UBIFS_DFS_DIR_NAME,
> + c->vi.ubi_num, c->vi.vol_id);
> +
> + if (n == UBIFS_DFS_DIR_LEN) {
> + /* The array size is too small */
> + ret = -EINVAL;
> + goto out_last;
Where is c->stats released in case of an error?
> + }
> +
> + c->kobj.kset = &ubifs_kset;
> + init_completion(&c->kobj_unregister);
> +
> +
> + ret = kobject_init_and_add(&c->kobj, &ubifs_sb_ktype, NULL,
> + "%s", dfs_dir_name);
> + if (ret)
> + goto out_put;
> +
> + return 0;
> +
> +out_put:
> + kobject_put(&c->kobj);
> + wait_for_completion(&c->kobj_unregister);
> +out_last:
> + ubifs_err(c, "cannot create sysfs entry for ubifs%d_%d, error %d\n",
> + c->vi.ubi_num, c->vi.vol_id, ret);
> + return ret;
> +}
> +
> +void ubifs_sysfs_unregister(struct ubifs_info *c)
> +{
> + kobject_del(&c->kobj);
> + kobject_put(&c->kobj);
> + wait_for_completion(&c->kobj_unregister);
> +
> + kfree(c->stats);
> +}
> +
> +int __init ubifs_sysfs_init(void)
> +{
> + int ret;
> +
> + kobject_set_name(&ubifs_kset.kobj, "ubifs");
> + ubifs_kset.kobj.parent = fs_kobj;
> + ret = kset_register(&ubifs_kset);
> +
> + return ret;
> +}
> +
> +void ubifs_sysfs_exit(void)
> +{
> + kset_unregister(&ubifs_kset);
> +}
> diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
> new file mode 100644
> index 000000000000..a10a82585af8
> --- /dev/null
> +++ b/fs/ubifs/sysfs.h
Do we really need a new header file?
Usually most run-time stuff of UBIFS is part of ubifs.h.
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * This file is part of UBIFS.
> + *
> + * Copyright (C) 2021 Cisco Systems
> + *
> + * Author: Stefan Schaeckeler
> + */
> +
> +#ifndef __UBIFS_SYSFS_H__
> +#define __UBIFS_SYSFS_H__
> +
> +struct ubifs_info;
> +
> +/*
> + * The UBIFS sysfs directory name pattern and maximum name length (3 for "ubi"
> + * + 1 for "_" and plus 2x2 for 2 UBI numbers and 1 for the trailing zero byte.
> + */
> +#define UBIFS_DFS_DIR_NAME "ubi%d_%d"
> +#define UBIFS_DFS_DIR_LEN (3 + 1 + 2*2 + 1)
> +
> +/**
> + * ubifs_stats_info - per-FS statistics information.
> + * @magic_errors: number of bad magic numbers (will be reset with a new mount).
> + * @node_errors: number of bad nodes (will be reset with a new mount).
> + * @crc_errors: number of bad crcs (will be reset with a new mount).
> + */
> +struct ubifs_stats_info {
> + unsigned int magic_errors;
> + unsigned int node_errors;
> + unsigned int crc_errors;
> +};
> +
> +int ubifs_sysfs_init(void);
> +void ubifs_sysfs_exit(void);
> +int ubifs_sysfs_register(struct ubifs_info *c);
> +void ubifs_sysfs_unregister(struct ubifs_info *c);
> +
> +#endif
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index c38066ce9ab0..bfc0f20b41a1 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -27,12 +27,15 @@
> #include <linux/security.h>
> #include <linux/xattr.h>
> #include <linux/random.h>
> +#include <linux/sysfs.h>
> +#include <linux/completion.h>
> #include <crypto/hash_info.h>
> #include <crypto/hash.h>
> #include <crypto/algapi.h>
>
> #include <linux/fscrypt.h>
>
> +#include "sysfs.h"
> #include "ubifs-media.h"
>
> /* Version of this UBIFS implementation */
> @@ -1251,6 +1254,10 @@ struct ubifs_debug_info;
> * @mount_opts: UBIFS-specific mount options
> *
> * @dbg: debugging-related information
> + * @stats: statistics exported over sysfs
> + *
> + * @kobj: kobject for /sys/fs/ubifs/
> + * @kobj_unregister: completion to unregister sysfs kobject
> */
> struct ubifs_info {
> struct super_block *vfs_sb;
> @@ -1286,6 +1293,9 @@ struct ubifs_info {
> spinlock_t cs_lock;
> wait_queue_head_t cmt_wq;
>
> + struct kobject kobj;
> + struct completion kobj_unregister;
> +
> unsigned int big_lpt:1;
> unsigned int space_fixup:1;
> unsigned int double_hash:1;
> @@ -1493,6 +1503,7 @@ struct ubifs_info {
> struct ubifs_mount_opts mount_opts;
>
> struct ubifs_debug_info *dbg;
> + struct ubifs_stats_info *stats;
> };
>
> extern struct list_head ubifs_infos;
> --
> 2.32.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
2021-10-03 19:58 ` Richard Weinberger
@ 2021-10-04 5:57 ` Stefan Schaeckeler
-1 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-10-04 5:57 UTC (permalink / raw)
To: richard; +Cc: linux-mtd, linux-kernel, sschaeck
Hello Richard,
> > Not all ubifs filesystem errors are propagated to userspace.
> >
> > Export bad magic, bad node and crc errors via sysfs. This allows userspace
> > to notice filesystem errors:
> >
> > /sys/fs/ubifs/ubiX_Y/errors_magic
> > /sys/fs/ubifs/ubiX_Y/errors_node
> > /sys/fs/ubifs/ubiX_Y/errors_crc
> >
> > The counters are reset to 0 with a remount. Writing anything into the
> > counters also clears them.
>
> I think this is a nice feature. Thanks for implementing it.
> Please see some minor nits below.
>
> Is there a specific reason why you didn't implement it via UBIFS's debugfs interface?
These error counters are not meant for aiding debugging but for userspace to
monitor the sanity of the filesystem. Also ext4 exports error counters via
sysfs - it's probably a good idea to be consistent with them.
$ dir /sys/fs/ext4/sdb2/*error*
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/errors_count
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_block
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_errcode
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_func
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_ino
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_line
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_time
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_block
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_errcode
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_func
-r--r--r-- 1 root root 4096 Oct 3 13:47 /sys/fs/ext4/sdb2/last_error_ino
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_line
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_time
--w------- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/trigger_fs_error
> sysfs is ABI, so we cannot change much anymore.
Judging by the filesystem permissions above, ext4 does not seem to allow
resetting error counters. If you worry about unchangable ABIs then we could
play it safe and don't support write callbacks for resetting the error counters
in an initial version of the ubifs sysfs. What do you think?
When we are at it, in the current code, writing anything into a sysfs node
resets the corresponding counter. One could fine-tune that to set the counter
to whatever non-negative integer is passed.
> > + if (c->stats)
> > + c->stats->magic_errors++;
>
> Please wrap this into a helper function.
Ack.
> > + if (c->stats)
> > + c->stats->node_errors++;
>
> Same.
Ack.
> > + if (c->stats)
> > + c->stats->crc_errors++;
>
> Same.
Ack.
> > +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
> > +
> > +UBIFS_ATTR_FUNC(errors_magic, 0644);
> > +UBIFS_ATTR_FUNC(errors_crc, 0644);
> > +UBIFS_ATTR_FUNC(errors_node, 0644);
>
> I'm not sure whether everyone should be allowed to read these stats.
> dmesg is also restricted these days. An unprivileged user should not see the
> errors he can indirectly trigger.
I don't mind 600, but having error counters world-readable is consistent with
ext4 (see dir above) and so I suggest to keep 644.
> > + case attr_errors_crc:
> > + return snprintf(buf, PAGE_SIZE, "%u\n",
> > + sbi->stats->crc_errors);
>
> Please use sysfs_emit().
Ack.
> > + if (n == UBIFS_DFS_DIR_LEN) {
> > + /* The array size is too small */
> > + ret = -EINVAL;
> > + goto out_last;
>
> Where is c->stats released in case of an error?
My fault. Will be fixed.
> > diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
> > new file mode 100644
> > index 000000000000..a10a82585af8
> > --- /dev/null
> > +++ b/fs/ubifs/sysfs.h
>
> Do we really need a new header file?
> Usually most run-time stuff of UBIFS is part of ubifs.h.
I wanted to be consistent with debugfs where fs/ubifs/debug.c comes with its
own header fs/ubifs/debug.h.
I'll send out a new patch once we agree on all changes. To recap:
- write callbacks: do we remove them? If not, do we keep them as is or do we
fine-tine them by letting a non-negative integer set the counter?
- 644 (world-readable) counters to be consistent with ext4?
- keep sysfs.h to be consistent with debugfs?
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
@ 2021-10-04 5:57 ` Stefan Schaeckeler
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Schaeckeler @ 2021-10-04 5:57 UTC (permalink / raw)
To: richard; +Cc: linux-mtd, linux-kernel, sschaeck
Hello Richard,
> > Not all ubifs filesystem errors are propagated to userspace.
> >
> > Export bad magic, bad node and crc errors via sysfs. This allows userspace
> > to notice filesystem errors:
> >
> > /sys/fs/ubifs/ubiX_Y/errors_magic
> > /sys/fs/ubifs/ubiX_Y/errors_node
> > /sys/fs/ubifs/ubiX_Y/errors_crc
> >
> > The counters are reset to 0 with a remount. Writing anything into the
> > counters also clears them.
>
> I think this is a nice feature. Thanks for implementing it.
> Please see some minor nits below.
>
> Is there a specific reason why you didn't implement it via UBIFS's debugfs interface?
These error counters are not meant for aiding debugging but for userspace to
monitor the sanity of the filesystem. Also ext4 exports error counters via
sysfs - it's probably a good idea to be consistent with them.
$ dir /sys/fs/ext4/sdb2/*error*
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/errors_count
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_block
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_errcode
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_func
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_ino
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_line
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_time
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_block
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_errcode
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_func
-r--r--r-- 1 root root 4096 Oct 3 13:47 /sys/fs/ext4/sdb2/last_error_ino
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_line
-r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_time
--w------- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/trigger_fs_error
> sysfs is ABI, so we cannot change much anymore.
Judging by the filesystem permissions above, ext4 does not seem to allow
resetting error counters. If you worry about unchangable ABIs then we could
play it safe and don't support write callbacks for resetting the error counters
in an initial version of the ubifs sysfs. What do you think?
When we are at it, in the current code, writing anything into a sysfs node
resets the corresponding counter. One could fine-tune that to set the counter
to whatever non-negative integer is passed.
> > + if (c->stats)
> > + c->stats->magic_errors++;
>
> Please wrap this into a helper function.
Ack.
> > + if (c->stats)
> > + c->stats->node_errors++;
>
> Same.
Ack.
> > + if (c->stats)
> > + c->stats->crc_errors++;
>
> Same.
Ack.
> > +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
> > +
> > +UBIFS_ATTR_FUNC(errors_magic, 0644);
> > +UBIFS_ATTR_FUNC(errors_crc, 0644);
> > +UBIFS_ATTR_FUNC(errors_node, 0644);
>
> I'm not sure whether everyone should be allowed to read these stats.
> dmesg is also restricted these days. An unprivileged user should not see the
> errors he can indirectly trigger.
I don't mind 600, but having error counters world-readable is consistent with
ext4 (see dir above) and so I suggest to keep 644.
> > + case attr_errors_crc:
> > + return snprintf(buf, PAGE_SIZE, "%u\n",
> > + sbi->stats->crc_errors);
>
> Please use sysfs_emit().
Ack.
> > + if (n == UBIFS_DFS_DIR_LEN) {
> > + /* The array size is too small */
> > + ret = -EINVAL;
> > + goto out_last;
>
> Where is c->stats released in case of an error?
My fault. Will be fixed.
> > diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
> > new file mode 100644
> > index 000000000000..a10a82585af8
> > --- /dev/null
> > +++ b/fs/ubifs/sysfs.h
>
> Do we really need a new header file?
> Usually most run-time stuff of UBIFS is part of ubifs.h.
I wanted to be consistent with debugfs where fs/ubifs/debug.c comes with its
own header fs/ubifs/debug.h.
I'll send out a new patch once we agree on all changes. To recap:
- write callbacks: do we remove them? If not, do we keep them as is or do we
fine-tine them by letting a non-negative integer set the counter?
- 644 (world-readable) counters to be consistent with ext4?
- keep sysfs.h to be consistent with debugfs?
Stefan
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
2021-10-04 5:57 ` Stefan Schaeckeler
@ 2021-10-09 20:48 ` Richard Weinberger
-1 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-10-09 20:48 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel, Stefan Schaeckeler
Stefan,
----- Ursprüngliche Mail -----
> Von: "schaecsn" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Stefan Schaeckeler"
> <sschaeck@cisco.com>
> Gesendet: Montag, 4. Oktober 2021 07:57:58
> Betreff: Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
> Hello Richard,
>
>> > Not all ubifs filesystem errors are propagated to userspace.
>> >
>> > Export bad magic, bad node and crc errors via sysfs. This allows userspace
>> > to notice filesystem errors:
>> >
>> > /sys/fs/ubifs/ubiX_Y/errors_magic
>> > /sys/fs/ubifs/ubiX_Y/errors_node
>> > /sys/fs/ubifs/ubiX_Y/errors_crc
>> >
>> > The counters are reset to 0 with a remount. Writing anything into the
>> > counters also clears them.
>>
>> I think this is a nice feature. Thanks for implementing it.
>> Please see some minor nits below.
>>
>> Is there a specific reason why you didn't implement it via UBIFS's debugfs
>> interface?
>
> These error counters are not meant for aiding debugging but for userspace to
> monitor the sanity of the filesystem. Also ext4 exports error counters via
> sysfs - it's probably a good idea to be consistent with them.
>
> $ dir /sys/fs/ext4/sdb2/*error*
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/errors_count
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_block
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_errcode
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_func
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_ino
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_line
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_time
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_block
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_errcode
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_func
> -r--r--r-- 1 root root 4096 Oct 3 13:47 /sys/fs/ext4/sdb2/last_error_ino
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_line
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_time
> --w------- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/trigger_fs_error
ext4 is not the reference design for filesystems in Linux.
e.g. btrfs has an ioctl for such stats.
>
>> sysfs is ABI, so we cannot change much anymore.
>
> Judging by the filesystem permissions above, ext4 does not seem to allow
> resetting error counters. If you worry about unchangable ABIs then we could
> play it safe and don't support write callbacks for resetting the error counters
> in an initial version of the ubifs sysfs. What do you think?
Ack.
> When we are at it, in the current code, writing anything into a sysfs node
> resets the corresponding counter. One could fine-tune that to set the counter
> to whatever non-negative integer is passed.
>
>
>> > + if (c->stats)
>> > + c->stats->magic_errors++;
>>
>> Please wrap this into a helper function.
>
> Ack.
>
>
>> > + if (c->stats)
>> > + c->stats->node_errors++;
>>
>> Same.
>
> Ack.
>
>
>> > + if (c->stats)
>> > + c->stats->crc_errors++;
>>
>> Same.
>
> Ack.
>
>
>> > +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
>> > +
>> > +UBIFS_ATTR_FUNC(errors_magic, 0644);
>> > +UBIFS_ATTR_FUNC(errors_crc, 0644);
>> > +UBIFS_ATTR_FUNC(errors_node, 0644);
>>
>> I'm not sure whether everyone should be allowed to read these stats.
>> dmesg is also restricted these days. An unprivileged user should not see the
>> errors he can indirectly trigger.
>
> I don't mind 600, but having error counters world-readable is consistent with
> ext4 (see dir above) and so I suggest to keep 644.
>
Ok.
>> > + case attr_errors_crc:
>> > + return snprintf(buf, PAGE_SIZE, "%u\n",
>> > + sbi->stats->crc_errors);
>>
>> Please use sysfs_emit().
>
> Ack.
>
>
>> > + if (n == UBIFS_DFS_DIR_LEN) {
>> > + /* The array size is too small */
>> > + ret = -EINVAL;
>> > + goto out_last;
>>
>> Where is c->stats released in case of an error?
>
> My fault. Will be fixed.
>
>
>> > diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
>> > new file mode 100644
>> > index 000000000000..a10a82585af8
>> > --- /dev/null
>> > +++ b/fs/ubifs/sysfs.h
>>
>> Do we really need a new header file?
>> Usually most run-time stuff of UBIFS is part of ubifs.h.
>
> I wanted to be consistent with debugfs where fs/ubifs/debug.c comes with its
> own header fs/ubifs/debug.h.
debug.h is not just about debugfs. It is about debugging/developing UBIFS.
That's why it is kind of special.
>
> I'll send out a new patch once we agree on all changes. To recap:
>
> - write callbacks: do we remove them? If not, do we keep them as is or do we
> fine-tine them by letting a non-negative integer set the counter?
I'd go for read-only first.
> - 644 (world-readable) counters to be consistent with ext4?
Ack.
> - keep sysfs.h to be consistent with debugfs?
Please remove sysfs.h.
Thanks,
//richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
@ 2021-10-09 20:48 ` Richard Weinberger
0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2021-10-09 20:48 UTC (permalink / raw)
To: schaecsn; +Cc: linux-mtd, linux-kernel, Stefan Schaeckeler
Stefan,
----- Ursprüngliche Mail -----
> Von: "schaecsn" <schaecsn@gmx.net>
> An: "richard" <richard@nod.at>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Stefan Schaeckeler"
> <sschaeck@cisco.com>
> Gesendet: Montag, 4. Oktober 2021 07:57:58
> Betreff: Re: [PATCH 1/1] ubifs: ubifs to export filesystem error counters
> Hello Richard,
>
>> > Not all ubifs filesystem errors are propagated to userspace.
>> >
>> > Export bad magic, bad node and crc errors via sysfs. This allows userspace
>> > to notice filesystem errors:
>> >
>> > /sys/fs/ubifs/ubiX_Y/errors_magic
>> > /sys/fs/ubifs/ubiX_Y/errors_node
>> > /sys/fs/ubifs/ubiX_Y/errors_crc
>> >
>> > The counters are reset to 0 with a remount. Writing anything into the
>> > counters also clears them.
>>
>> I think this is a nice feature. Thanks for implementing it.
>> Please see some minor nits below.
>>
>> Is there a specific reason why you didn't implement it via UBIFS's debugfs
>> interface?
>
> These error counters are not meant for aiding debugging but for userspace to
> monitor the sanity of the filesystem. Also ext4 exports error counters via
> sysfs - it's probably a good idea to be consistent with them.
>
> $ dir /sys/fs/ext4/sdb2/*error*
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/errors_count
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_block
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_errcode
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_func
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_ino
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_line
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/first_error_time
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_block
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_errcode
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_func
> -r--r--r-- 1 root root 4096 Oct 3 13:47 /sys/fs/ext4/sdb2/last_error_ino
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_line
> -r--r--r-- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/last_error_time
> --w------- 1 root root 4096 Oct 3 13:46 /sys/fs/ext4/sdb2/trigger_fs_error
ext4 is not the reference design for filesystems in Linux.
e.g. btrfs has an ioctl for such stats.
>
>> sysfs is ABI, so we cannot change much anymore.
>
> Judging by the filesystem permissions above, ext4 does not seem to allow
> resetting error counters. If you worry about unchangable ABIs then we could
> play it safe and don't support write callbacks for resetting the error counters
> in an initial version of the ubifs sysfs. What do you think?
Ack.
> When we are at it, in the current code, writing anything into a sysfs node
> resets the corresponding counter. One could fine-tune that to set the counter
> to whatever non-negative integer is passed.
>
>
>> > + if (c->stats)
>> > + c->stats->magic_errors++;
>>
>> Please wrap this into a helper function.
>
> Ack.
>
>
>> > + if (c->stats)
>> > + c->stats->node_errors++;
>>
>> Same.
>
> Ack.
>
>
>> > + if (c->stats)
>> > + c->stats->crc_errors++;
>>
>> Same.
>
> Ack.
>
>
>> > +#define UBIFS_ATTR_FUNC(_name, _mode) UBIFS_ATTR(_name, _mode, _name)
>> > +
>> > +UBIFS_ATTR_FUNC(errors_magic, 0644);
>> > +UBIFS_ATTR_FUNC(errors_crc, 0644);
>> > +UBIFS_ATTR_FUNC(errors_node, 0644);
>>
>> I'm not sure whether everyone should be allowed to read these stats.
>> dmesg is also restricted these days. An unprivileged user should not see the
>> errors he can indirectly trigger.
>
> I don't mind 600, but having error counters world-readable is consistent with
> ext4 (see dir above) and so I suggest to keep 644.
>
Ok.
>> > + case attr_errors_crc:
>> > + return snprintf(buf, PAGE_SIZE, "%u\n",
>> > + sbi->stats->crc_errors);
>>
>> Please use sysfs_emit().
>
> Ack.
>
>
>> > + if (n == UBIFS_DFS_DIR_LEN) {
>> > + /* The array size is too small */
>> > + ret = -EINVAL;
>> > + goto out_last;
>>
>> Where is c->stats released in case of an error?
>
> My fault. Will be fixed.
>
>
>> > diff --git a/fs/ubifs/sysfs.h b/fs/ubifs/sysfs.h
>> > new file mode 100644
>> > index 000000000000..a10a82585af8
>> > --- /dev/null
>> > +++ b/fs/ubifs/sysfs.h
>>
>> Do we really need a new header file?
>> Usually most run-time stuff of UBIFS is part of ubifs.h.
>
> I wanted to be consistent with debugfs where fs/ubifs/debug.c comes with its
> own header fs/ubifs/debug.h.
debug.h is not just about debugfs. It is about debugging/developing UBIFS.
That's why it is kind of special.
>
> I'll send out a new patch once we agree on all changes. To recap:
>
> - write callbacks: do we remove them? If not, do we keep them as is or do we
> fine-tine them by letting a non-negative integer set the counter?
I'd go for read-only first.
> - 644 (world-readable) counters to be consistent with ext4?
Ack.
> - keep sysfs.h to be consistent with debugfs?
Please remove sysfs.h.
Thanks,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-10-09 20:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 21:40 [PATCH 0/1] ubifs: ubifs to export filesystem error counters Stefan Schaeckeler
2021-09-07 21:40 ` Stefan Schaeckeler
2021-09-07 21:40 ` [PATCH 1/1] " Stefan Schaeckeler
2021-09-07 21:40 ` Stefan Schaeckeler
2021-10-03 19:58 ` Richard Weinberger
2021-10-03 19:58 ` Richard Weinberger
2021-10-04 5:57 ` Stefan Schaeckeler
2021-10-04 5:57 ` Stefan Schaeckeler
2021-10-09 20:48 ` Richard Weinberger
2021-10-09 20:48 ` Richard Weinberger
2021-09-20 2:48 ` [PATCH 0/1] " Stefan Schaeckeler
2021-09-20 2:48 ` Stefan Schaeckeler
2021-09-20 21:57 ` Richard Weinberger
2021-09-20 21:57 ` Richard Weinberger
2021-10-02 21:12 ` Stefan Schaeckeler
2021-10-02 21:12 ` Stefan Schaeckeler
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.