Re: [PATCH -next] md: simplify md_seq_ops

[Song Liu]

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