linux-kernel - Re: [PATCH -next] md: simplify md_seq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 25 Sep 2023 09:07:06 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     Song Liu <song@...nel.org>, Yu Kuai <yukuai1@...weicloud.com>
Cc:     Mariusz Tkaczyk <mariusz.tkaczyk@...ux.intel.com>,
        linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
        yi.zhang@...wei.com, yangerkun@...wei.com,
        "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH -next] md: simplify md_seq_ops

Hi,

在 2023/09/23 5:22, Song Liu 写道:
> On Mon, Sep 11, 2023 at 6:02 PM Yu Kuai <yukuai1@...weicloud.com> 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?

before this patch, in each loop:
- hold lock, get mddev, drop lock
- md_seq_show

and after this patch:
- hold lock in start, drop lock in stop
- lock is always held in each loop

And mariusz is concerned that lock time is increased and may cause some
performance regression.

We've discussed in slack, and decided to keep this behaviour. I'll
update this in v2.

Thanks,
Kuai

> 
> Hi Kuai,
> 
> This patch doesn't apply cleanly to md-next now. Please rebase and send v2.
> 
> Thanks,
> Song
> .
>