Messages in this thread Patch in this message | | | Subject | Re: [PATCH v3 2/2] md: simplify md_seq_ops | From | Yu Kuai <> | Date | Tue, 9 Jan 2024 15:48:25 +0800 |
| |
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@huaweicloud.com> wrote: >>> >>> From: Yu Kuai <yukuai3@huawei.com> >>> >>> 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@huawei.com> >> >> 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 >> >> . >> > > . >
| |