Re: [PATCH v3 2/2] md: simplify md_seq_ops

On Mon, Jan 8, 2024 at 11:48 PM Yu Kuai [off-list ref] wrote:

quoted

Hi,

在 2024/01/09 9:21, Yu Kuai 写道:

quoted

Hi,

在 2024/01/09 7:38, Song Liu 写道:

quoted

On Tue, Sep 26, 2023 at 11:19 PM Yu Kuai [off-list ref] wrote:

[...]

quoted

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);

I think something like the following is the right way to do this.

Thanks,
Song

diff --git i/drivers/md/md.c w/drivers/md/md.c
index 38a6767c65b1..14044febe009 100644
--- i/drivers/md/md.c
+++ w/drivers/md/md.c

@@ -8215,20 +8215,8 @@ static int status_resync(struct seq_file *seq,

struct mddev *mddev)
  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);
+       return seq_list_start_head(&all_mddevs, *pos);

  }

  static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)

@@ -8243,12 +8231,31 @@ static void md_seq_stop(struct seq_file *seq, void *v)
         spin_unlock(&all_mddevs_lock);
  }

+static void md_seq_print_header(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");
+       seq->poll_event = atomic_read(&md_event_count);
+}
+
  static int md_seq_show(struct seq_file *seq, void *v)
  {
         struct mddev *mddev = list_entry(v, struct mddev, all_mddevs);
         sector_t sectors;
         struct md_rdev *rdev;

+       if (v == &all_mddevs) {
+               md_seq_print_header(seq);