[RFC 0/1] ubi: An interface for dump the mapping between LEBs and PEBs

[RFC 0/1] ubi: An interface for dump the mapping between LEBs and PEBs

[ZhaoLong Wang]

When a metadata exception occurs on a device using UBIFS, we can easily
obtain the LEB number of the data exception from the log, but the PEB
number corresponding to the LEB cannot be obtained.

In some production lines, the NAND flash produced by the NAND flash
cannot provide reliable ECC error correction capabilities. The file
system damage is always related to the flash eraseblock exception.
Therefore, I hope that there is a way to quickly obtain the mapping
between LEBs and PEBs in the user space. In this way, the tool of 
user mode can be used to debug possible faulty physical blocks
more conveniently.

Currently, the implementation of this interface is rough. It does
not obtain necessary locks when traversing the entire eba_tbl. Therefore,
this operation race with the volume update process (such as resize and
wear-leveling). As a result, the user mode obtains incorrect mapping
information. However, in practice, when the development and O&M need
to use this interface, frequent volume management operations do not
occur. Therefore, this problem may be tolerated.

I hope the experts will give valuable comments on this patch. If this
interface is acceptable, I'll make it perform as well as I can.

ZhaoLong Wang (1):
  ubi: An interface is added for dump the mapping between LEBs and PEBs

 drivers/mtd/ubi/debug.c | 104 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/eba.c   |  24 ----------
 drivers/mtd/ubi/ubi.h   |  24 ++++++++++
 3 files changed, 128 insertions(+), 24 deletions(-)

-- 
2.31.1

[RFC 1/1] ubi: An interface is added for dump the mapping between LEBs and PEBs

[ZhaoLong Wang]

A debugfs interface named "detailed_eba_table_info" is added to view
the PEB and LEB mapping information of all volumes on a UBI device.

$ cat /sys/kernel/debug/ubi/ubi1/detailed_eba_table_info

=========   vol_name:"test_volA",id:2   ========
logical_block_number    physical_block_number
0                        274
1                        275
2                        276
3                        277
4                        278
......
=========   vol_name:"test_volB",id:3   ========
logical_block_number    physical_block_number
0                        619
1                        613
2                        614
3                       (unmaped)
4                        622
......

Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com>
---
 drivers/mtd/ubi/debug.c | 104 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/eba.c   |  24 ----------
 drivers/mtd/ubi/ubi.h   |  24 ++++++++++
 3 files changed, 128 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
index 27168f511d6d..c6be9450b509 100644
--- a/drivers/mtd/ubi/debug.c
+++ b/drivers/mtd/ubi/debug.c
@@ -386,6 +386,7 @@ static const struct file_operations dfs_fops = {
 	.owner  = THIS_MODULE,
 };
 
+
 /* As long as the position is less then that total number of erase blocks,
  * we still have more to print.
  */
@@ -493,6 +494,106 @@ static const struct file_operations eraseblk_count_fops = {
 	.release = eraseblk_count_release,
 };
 
+static void *eba_table_seq_start(struct seq_file *s, loff_t *pos)
+{
+	int volumes_length = UBI_MAX_VOLUMES + UBI_INT_VOL_COUNT;
+
+	if (*pos >= 0 && *pos < volumes_length)
+		return pos;
+
+	return NULL;
+}
+
+static void *eba_table_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	int volumes_length = UBI_MAX_VOLUMES + UBI_INT_VOL_COUNT;
+
+	(*pos)++;
+
+	if (*pos >= 0 && *pos < volumes_length)
+		return pos;
+
+	return NULL;
+}
+
+static void eba_table_seq_stop(struct seq_file *s, void *v)
+{
+}
+
+static int eba_table_seq_show(struct seq_file *s, void *iter)
+{
+	struct ubi_device *ubi = s->private;
+	int *vol_idx  = iter;
+	struct ubi_volume *vol;
+	int lnum, pnum, vol_id;
+
+	vol = ubi->volumes[*vol_idx];
+	if (vol == NULL)
+		return 0;
+
+	vol_id = vol->vol_id;
+	seq_printf(s, "=========   vol_name:\"%s\",id:%d   ========\n", vol->name, vol_id);
+	seq_puts(s, "logical_block_number\tphysical_block_number\n");
+	spin_lock(&ubi->volumes_lock);
+	for (lnum = 0; lnum < vol->reserved_pebs; ++lnum) {
+		pnum = vol->eba_tbl->entries[lnum].pnum;
+		if (pnum >= 0)
+			seq_printf(s, "%-22d\t %-11d\n", lnum, pnum);
+		else
+			seq_printf(s, "%-22d\t(unmaped)\n", lnum);
+	}
+	spin_unlock(&ubi->volumes_lock);
+
+	return 0;
+}
+
+static const struct seq_operations eba_table_seq_ops = {
+	.start = eba_table_seq_start,
+	.next = eba_table_seq_next,
+	.stop = eba_table_seq_stop,
+	.show = eba_table_seq_show
+};
+
+static int eba_table_open(struct inode *inode, struct file *f)
+{
+	struct seq_file *s;
+	int err;
+
+	err = seq_open(f, &eba_table_seq_ops);
+	if (err)
+		return err;
+
+	s = f->private_data;
+	s->private = ubi_get_device((unsigned long)inode->i_private);
+
+	if (!s->private)
+		return -ENODEV;
+	else
+		return 0;
+}
+
+static int eba_table_release(struct inode *inode, struct file *f)
+{
+	struct seq_file *s = f->private_data;
+	struct ubi_device *ubi = s->private;
+
+	ubi_put_device(ubi);
+
+	return seq_release(inode, f);
+}
+
+/* File operations for UBI debugfs files which to read
+ * eba_tbl to help developer get the map status of ubi
+ * volumes
+ */
+static const struct file_operations eba_table_fops = {
+	.owner = THIS_MODULE,
+	.open = eba_table_open,
+	.read = seq_read,
+	.llseek = no_llseek,
+	.release = eba_table_release,
+};
+
 /**
  * ubi_debugfs_init_dev - initialize debugfs for an UBI device.
  * @ubi: UBI device description object
@@ -556,6 +657,9 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
 						   mode, d->dfs_dir,
 						   (void *)ubi_num, &dfs_fops);
 
+	debugfs_create_file("detailed_eba_table_info", S_IRUSR, d->dfs_dir,
+			    (void *)ubi_num, &eba_table_fops);
+
 	debugfs_create_file("detailed_erase_block_info", S_IRUSR, d->dfs_dir,
 			    (void *)ubi_num, &eraseblk_count_fops);
 
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 655ff41863e2..c856ea73a67c 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -36,30 +36,6 @@
 /* Number of physical eraseblocks reserved for atomic LEB change operation */
 #define EBA_RESERVED_PEBS 1
 
-/**
- * struct ubi_eba_entry - structure encoding a single LEB -> PEB association
- * @pnum: the physical eraseblock number attached to the LEB
- *
- * This structure is encoding a LEB -> PEB association. Note that the LEB
- * number is not stored here, because it is the index used to access the
- * entries table.
- */
-struct ubi_eba_entry {
-	int pnum;
-};
-
-/**
- * struct ubi_eba_table - LEB -> PEB association information
- * @entries: the LEB to PEB mapping (one entry per LEB).
- *
- * This structure is private to the EBA logic and should be kept here.
- * It is encoding the LEB to PEB association table, and is subject to
- * changes.
- */
-struct ubi_eba_table {
-	struct ubi_eba_entry *entries;
-};
-
 /**
  * ubi_next_sqnum - get next sequence number.
  * @ubi: UBI device description object
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index c8f1bd4fa100..1457657901df 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -153,6 +153,30 @@ enum {
 	POWER_CUT_VID_WRITE = 0x02,
 };
 
+/**
+ * struct ubi_eba_entry - structure encoding a single LEB -> PEB association
+ * @pnum: the physical eraseblock number attached to the LEB
+ *
+ * This structure is encoding a LEB -> PEB association. Note that the LEB
+ * number is not stored here, because it is the index used to access the
+ * entries table.
+ */
+struct ubi_eba_entry {
+	int pnum;
+};
+
+/**
+ * struct ubi_eba_table - LEB -> PEB association information
+ * @entries: the LEB to PEB mapping (one entry per LEB).
+ *
+ * This structure is private to the EBA logic and should be kept here.
+ * It is encoding the LEB to PEB association table, and is subject to
+ * changes.
+ */
+struct ubi_eba_table {
+	struct ubi_eba_entry *entries;
+};
+
 /**
  * struct ubi_vid_io_buf - VID buffer used to read/write VID info to/from the
  *			   flash.
-- 
2.31.1

Re: [RFC 1/1] ubi: An interface is added for dump the mapping between LEBs and PEBs

[Zhihao Cheng]

在 2023/7/22 18:23, ZhaoLong Wang 写道:
> A debugfs interface named "detailed_eba_table_info" is added to view
> the PEB and LEB mapping information of all volumes on a UBI device.
> 
> $ cat /sys/kernel/debug/ubi/ubi1/detailed_eba_table_info
> 
> =========   vol_name:"test_volA",id:2   ========
> logical_block_number    physical_block_number
> 0                        274
> 1                        275
> 2                        276
> 3                        277
> 4                        278
> ......
> =========   vol_name:"test_volB",id:3   ========
> logical_block_number    physical_block_number
> 0                        619
> 1                        613
> 2                        614
> 3                       (unmaped)
> 4                        622
> ......
> 
> Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com>
> ---
>   drivers/mtd/ubi/debug.c | 104 ++++++++++++++++++++++++++++++++++++++++
>   drivers/mtd/ubi/eba.c   |  24 ----------
>   drivers/mtd/ubi/ubi.h   |  24 ++++++++++
>   3 files changed, 128 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
> index 27168f511d6d..c6be9450b509 100644
> --- a/drivers/mtd/ubi/debug.c
> +++ b/drivers/mtd/ubi/debug.c
> @@ -386,6 +386,7 @@ static const struct file_operations dfs_fops = {
>   	.owner  = THIS_MODULE,
>   };
>   
> +
>   /* As long as the position is less then that total number of erase blocks,
>    * we still have more to print.
>    */
> @@ -493,6 +494,106 @@ static const struct file_operations eraseblk_count_fops = {
>   	.release = eraseblk_count_release,
>   };
>   
> +static void *eba_table_seq_start(struct seq_file *s, loff_t *pos)
> +{
> +	int volumes_length = UBI_MAX_VOLUMES + UBI_INT_VOL_COUNT;
> +
> +	if (*pos >= 0 && *pos < volumes_length)
> +		return pos;
> +
> +	return NULL;
> +}
> +
> +static void *eba_table_seq_next(struct seq_file *s, void *v, loff_t *pos)
> +{
> +	int volumes_length = UBI_MAX_VOLUMES + UBI_INT_VOL_COUNT;
> +
> +	(*pos)++;
> +
> +	if (*pos >= 0 && *pos < volumes_length)
> +		return pos;
> +
> +	return NULL;
> +}
> +
> +static void eba_table_seq_stop(struct seq_file *s, void *v)
> +{
> +}
> +
> +static int eba_table_seq_show(struct seq_file *s, void *iter)
> +{
> +	struct ubi_device *ubi = s->private;
> +	int *vol_idx  = iter;
> +	struct ubi_volume *vol;
> +	int lnum, pnum, vol_id;
> +
> +	vol = ubi->volumes[*vol_idx];
> +	if (vol == NULL)
> +		return 0;
> +
> +	vol_id = vol->vol_id;
> +	seq_printf(s, "=========   vol_name:\"%s\",id:%d   ========\n", vol->name, vol_id);
> +	seq_puts(s, "logical_block_number\tphysical_block_number\n");
> +	spin_lock(&ubi->volumes_lock);
> +	for (lnum = 0; lnum < vol->reserved_pebs; ++lnum) {
> +		pnum = vol->eba_tbl->entries[lnum].pnum;
> +		if (pnum >= 0)
> +			seq_printf(s, "%-22d\t %-11d\n", lnum, pnum);
> +		else
> +			seq_printf(s, "%-22d\t(unmaped)\n", lnum);
> +	}
> +	spin_unlock(&ubi->volumes_lock);
> +
> +	return 0;
> +}

Mote, this traversal of 'vol->eba_tbl->entries' depends on 
https://patchwork.ozlabs.org/project/linux-mtd/patch/20230504025354.3595768-1-wangzhaolong1@huawei.com/.

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> +
> +static const struct seq_operations eba_table_seq_ops = {
> +	.start = eba_table_seq_start,
> +	.next = eba_table_seq_next,
> +	.stop = eba_table_seq_stop,
> +	.show = eba_table_seq_show
> +};
> +
> +static int eba_table_open(struct inode *inode, struct file *f)
> +{
> +	struct seq_file *s;
> +	int err;
> +
> +	err = seq_open(f, &eba_table_seq_ops);
> +	if (err)
> +		return err;
> +
> +	s = f->private_data;
> +	s->private = ubi_get_device((unsigned long)inode->i_private);
> +
> +	if (!s->private)
> +		return -ENODEV;
> +	else
> +		return 0;
> +}
> +
> +static int eba_table_release(struct inode *inode, struct file *f)
> +{
> +	struct seq_file *s = f->private_data;
> +	struct ubi_device *ubi = s->private;
> +
> +	ubi_put_device(ubi);
> +
> +	return seq_release(inode, f);
> +}
> +
> +/* File operations for UBI debugfs files which to read
> + * eba_tbl to help developer get the map status of ubi
> + * volumes
> + */
> +static const struct file_operations eba_table_fops = {
> +	.owner = THIS_MODULE,
> +	.open = eba_table_open,
> +	.read = seq_read,
> +	.llseek = no_llseek,
> +	.release = eba_table_release,
> +};
> +
>   /**
>    * ubi_debugfs_init_dev - initialize debugfs for an UBI device.
>    * @ubi: UBI device description object
> @@ -556,6 +657,9 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
>   						   mode, d->dfs_dir,
>   						   (void *)ubi_num, &dfs_fops);
>   
> +	debugfs_create_file("detailed_eba_table_info", S_IRUSR, d->dfs_dir,
> +			    (void *)ubi_num, &eba_table_fops);
> +
>   	debugfs_create_file("detailed_erase_block_info", S_IRUSR, d->dfs_dir,
>   			    (void *)ubi_num, &eraseblk_count_fops);
>   
> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
> index 655ff41863e2..c856ea73a67c 100644
> --- a/drivers/mtd/ubi/eba.c
> +++ b/drivers/mtd/ubi/eba.c
> @@ -36,30 +36,6 @@
>   /* Number of physical eraseblocks reserved for atomic LEB change operation */
>   #define EBA_RESERVED_PEBS 1
>   
> -/**
> - * struct ubi_eba_entry - structure encoding a single LEB -> PEB association
> - * @pnum: the physical eraseblock number attached to the LEB
> - *
> - * This structure is encoding a LEB -> PEB association. Note that the LEB
> - * number is not stored here, because it is the index used to access the
> - * entries table.
> - */
> -struct ubi_eba_entry {
> -	int pnum;
> -};
> -
> -/**
> - * struct ubi_eba_table - LEB -> PEB association information
> - * @entries: the LEB to PEB mapping (one entry per LEB).
> - *
> - * This structure is private to the EBA logic and should be kept here.
> - * It is encoding the LEB to PEB association table, and is subject to
> - * changes.
> - */
> -struct ubi_eba_table {
> -	struct ubi_eba_entry *entries;
> -};
> -
>   /**
>    * ubi_next_sqnum - get next sequence number.
>    * @ubi: UBI device description object
> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
> index c8f1bd4fa100..1457657901df 100644
> --- a/drivers/mtd/ubi/ubi.h
> +++ b/drivers/mtd/ubi/ubi.h
> @@ -153,6 +153,30 @@ enum {
>   	POWER_CUT_VID_WRITE = 0x02,
>   };
>   
> +/**
> + * struct ubi_eba_entry - structure encoding a single LEB -> PEB association
> + * @pnum: the physical eraseblock number attached to the LEB
> + *
> + * This structure is encoding a LEB -> PEB association. Note that the LEB
> + * number is not stored here, because it is the index used to access the
> + * entries table.
> + */
> +struct ubi_eba_entry {
> +	int pnum;
> +};
> +
> +/**
> + * struct ubi_eba_table - LEB -> PEB association information
> + * @entries: the LEB to PEB mapping (one entry per LEB).
> + *
> + * This structure is private to the EBA logic and should be kept here.
> + * It is encoding the LEB to PEB association table, and is subject to
> + * changes.
> + */
> +struct ubi_eba_table {
> +	struct ubi_eba_entry *entries;
> +};
> +
>   /**
>    * struct ubi_vid_io_buf - VID buffer used to read/write VID info to/from the
>    *			   flash.
>