Hi,

在 2023/12/07 4:38, Song Liu 写道:

On Wed, Dec 6, 2023 at 12:37 AM Yuya Fujita [off-list ref] wrote:

quoted

Do not unlock all_mddevs_lock in md_seq_show() and keep the lock until
md_seq_stop().

The list of all_mddevs may be deleted in md_seq_show() because
all_mddevs_lock is temporarily unlocked.
The list should not be deleted while iterating over all_mddevs.
(Just as the list should not be deleted during list_for_each_entry().)

Fixes: cf1b6d4441ff ("md: simplify md_seq_ops")
Signed-off-by: Yuya Fujita <redacted>

Looks reasonable to me.

Yu Kuai, what do you think about this?

I fail to understand what is the problem here if other mddev is deleted
from the list while all_mddevs_lock is released during md_seq_show(),
can you explain more?

Thanks,
Kuai

Thanks,
Song

quoted

---
  drivers/md/md.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index c94373d64f2c..97ef08608848 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c

@@ -8293,7 +8293,6 @@ static int md_seq_show(struct seq_file *seq, void *v)
         if (!mddev_get(mddev))
                 return 0;

-       spin_unlock(&all_mddevs_lock);
         spin_lock(&mddev->lock);
         if (mddev->pers || mddev->raid_disks || !list_empty(&mddev->disks)) {
                 seq_printf(seq, "%s : %sactive", mdname(mddev),

@@ -8364,7 +8363,6 @@ static int md_seq_show(struct seq_file *seq, void *v)
                 seq_printf(seq, "\n");
         }
         spin_unlock(&mddev->lock);
-       spin_lock(&all_mddevs_lock);
         if (atomic_dec_and_test(&mddev->active))
                 __mddev_put(mddev);

--

2.31.1

.