On Mon, Sep 11, 2023 at 6:02 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
[...]
> >> +static void *md_seq_start(struct seq_file *seq, loff_t *pos)
> >> +{
> >> + 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);
> >
> > I would prefer to increase "active" instead holding lock when enumerating over
> > the devices. the main reason is that parsing mdstat is implemented in mdadm, so
> > it could kind of blocker action- for example mdmon follows mdstat so it is read
> > frequently. The time of getting other actions done can highly increase because
> > every open or sysfs_read/write requires this lock.
Existing code holds pers_lock can seq_printf() in md_seq_show(). Do we see
issues with this?
Hi Kuai,
This patch doesn't apply cleanly to md-next now. Please rebase and send v2.
Thanks,
Song
