Hi,
在 2024/01/09 9:21, Yu Kuai 写道:
Hi,
在 2024/01/09 7:38, Song Liu 写道:
On Tue, Sep 26, 2023 at 11:19 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
From: Yu Kuai <yukuai3@xxxxxxxxxx>
Before this patch, the implementation is hacky and hard to understand:
1) md_seq_start set pos to 1;
2) md_seq_show found pos is 1, then print Personalities;
3) md_seq_next found pos is 1, then it update pos to the first mddev;
4) md_seq_show found pos is not 1 or 2, show mddev;
5) md_seq_next found pos is not 1 or 2, update pos to next mddev;
6) loop 4-5 until the last mddev, then md_seq_next update pos to 2;
7) md_seq_show found pos is 2, then print unused devices;
8) md_seq_next found pos is 2, stop;
This patch remove the magic value and use seq_list_start/next/stop()
directly, and move printing "Personalities" to md_seq_start(),
"unsed devices" to md_seq_stop():
1) md_seq_start print Personalities, and then set pos to first mddev;
2) md_seq_show show mddev;
3) md_seq_next update pos to next mddev;
4) loop 2-3 until the last mddev;
5) md_seq_stop print unsed devices;
Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>
Just realized this introduced a behavior change:
When there is not md devices, before this patch, we have
[root@eth50-1 ~]# cat /proc/mdstat
Personalities : [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
unused devices: <none>
After this patch, "cat /proc/mdstat" returns nothing. This causes
some confusion for users who want to read "Personalities" line,
for example, the mdadm test suite reads it.
I haven't figured out the best fix yet.
Yes, that's a problem. And after reviewing seq_read_iter() in detail, I
realize that I also can't use seq_printf() in m->op->start() directly,
because if seq buffer overflowed, md_seq_start() can be called more than
once.
I'll fix these problems soon.
How about following patch(already tested)?
Thanks,
Kuai
diff --git a/drivers/md/md.c b/drivers/md/md.c
index e351e6c51cc7..289d3d89e73d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8135,6 +8135,19 @@ static void status_unused(struct seq_file *seq)
seq_printf(seq, "\n");
}
+static void status_personalities(struct seq_file *seq)
+{
+ struct md_personality *pers;
+
+ seq_puts(seq, "Personalities : ");
+ spin_lock(&pers_lock);
+ list_for_each_entry(pers, &pers_list, list)
+ seq_printf(seq, "[%s] ", pers->name);
+
+ spin_unlock(&pers_lock);
+ seq_puts(seq, "\n");
+}
+
static int status_resync(struct seq_file *seq, struct mddev *mddev)
{
sector_t max_sectors, resync, res;
@@ -8273,43 +8286,53 @@ static int status_resync(struct seq_file *seq,
struct mddev *mddev)
return 1;
}
+#define MDDEV_NONE (void *)1
+
static void *md_seq_start(struct seq_file *seq, loff_t *pos)
__acquires(&all_mddevs_lock)
{
- struct md_personality *pers;
-
- seq_puts(seq, "Personalities : ");
- spin_lock(&pers_lock);
- list_for_each_entry(pers, &pers_list, list)
- seq_printf(seq, "[%s] ", pers->name);
-
- spin_unlock(&pers_lock);
- seq_puts(seq, "\n");
seq->poll_event = atomic_read(&md_event_count);
-
spin_lock(&all_mddevs_lock);
- return seq_list_start(&all_mddevs, *pos);
+ if (!list_empty(&all_mddevs))
+ return seq_list_start(&all_mddevs, *pos);
+ else if (*pos == 0)
+ return MDDEV_NONE;
+ else
+ return NULL;
}
static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
+ if (v == MDDEV_NONE) {
+ ++*pos;
+ return NULL;
+ }
+
return seq_list_next(v, &all_mddevs, pos);
}
static void md_seq_stop(struct seq_file *seq, void *v)
__releases(&all_mddevs_lock)
{
- status_unused(seq);
spin_unlock(&all_mddevs_lock);
}
static int md_seq_show(struct seq_file *seq, void *v)
{
- struct mddev *mddev = list_entry(v, struct mddev, all_mddevs);
+ struct mddev *mddev;
sector_t sectors;
struct md_rdev *rdev;
+ if (v == MDDEV_NONE) {
+ status_personalities(seq);
+ status_unused(seq);
+ return 0;
+ }
+
+ mddev = list_entry(v, struct mddev, all_mddevs);
+ if (mddev == list_first_entry(&all_mddevs, struct mddev,
all_mddevs))
+ status_personalities(seq);
if (!mddev_get(mddev))
return 0;
@@ -8385,6 +8408,10 @@ static int md_seq_show(struct seq_file *seq, void *v)
}
spin_unlock(&mddev->lock);
spin_lock(&all_mddevs_lock);
+
+ if (mddev == list_last_entry(&all_mddevs, struct mddev, all_mddevs))
+ status_unused(seq);
+
if (atomic_dec_and_test(&mddev->active))
__mddev_put(mddev);
Thanks,
Kuai
Thanks,
Song
.
.
