Received-SPF: pass (google.com: domain of linux-kernel+bounces-20529-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20230927061241.1552837-1-yukuai1@huaweicloud.com>
 <20230927061241.1552837-3-yukuai1@huaweicloud.com> <CAPhsuW6sdnJYtE+iy+x=C2qVKzeN18zibx+qQBF4Y=KRsAmTTg@mail.gmail.com>
 <b6a79bb8-e0fc-09b4-90e7-8112100a3fd0@huaweicloud.com> <753615cc-16d6-3c58-99ee-b5e1f0aa0cde@huaweicloud.com>
In-Reply-To: <753615cc-16d6-3c58-99ee-b5e1f0aa0cde@huaweicloud.com>
From: Song Liu <song@kernel.org>
Date: Tue, 9 Jan 2024 00:12:30 -0800
Message-ID: <CAPhsuW6Ekxu9kaxvi673MCr=nBCS78D613zuH6Kcg5Y4Hs=jDw@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] md: simplify md_seq_ops
To: Yu Kuai <yukuai1@huaweicloud.com>
Cc: mariusz.tkaczyk@linux.intel.com, xni@redhat.com, 
	linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org, yi.zhang@huawei.com, 
	yangerkun@huawei.com, "yukuai (C)" <yukuai3@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 8, 2024 at 11:48=E2=80=AFPM Yu Kuai <yukuai1@huaweicloud.com> w=
rote:
>
> Hi,
>
> =E5=9C=A8 2024/01/09 9:21, Yu Kuai =E5=86=99=E9=81=93:
> > Hi,
> >
> > =E5=9C=A8 2024/01/09 7:38, Song Liu =E5=86=99=E9=81=93:
> >> On Tue, Sep 26, 2023 at 11:19=E2=80=AFPM Yu Kuai <yukuai1@huaweicloud.=
com> wrote:
[...]
> 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 =3D 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 =3D=3D 0)
> +               return MDDEV_NONE;
> +       else
> +               return NULL;
>   }
>
>   static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
>   {
> +       if (v =3D=3D 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 =3D list_entry(v, struct mddev, all_mddevs);
> +       struct mddev *mddev;
>          sector_t sectors;
>          struct md_rdev *rdev;
>
> +       if (v =3D=3D MDDEV_NONE) {
> +               status_personalities(seq);
> +               status_unused(seq);
> +               return 0;
> +       }
> +
> +       mddev =3D list_entry(v, struct mddev, all_mddevs);
> +       if (mddev =3D=3D 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 =3D=3D list_last_entry(&all_mddevs, struct mddev, all_m=
ddevs))
> +               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 =3D 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 =3D atomic_read(&md_event_count);
+}
+
 static int md_seq_show(struct seq_file *seq, void *v)
 {
        struct mddev *mddev =3D list_entry(v, struct mddev, all_mddevs);
        sector_t sectors;
        struct md_rdev *rdev;

+       if (v =3D=3D &all_mddevs) {
+               md_seq_print_header(seq);
+               return 0;
+       }
+
        if (!mddev_get(mddev))
                return 0;