linux-kernel - Re: [PATCH 3/4] md: fix is_mddev

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]

Message-ID: <2acd0bd5-f514-6c59-e90e-18a6d46174c2@huaweicloud.com>
Date: Thu, 17 Apr 2025 09:47:05 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Xiao Ni <xni@...hat.com>, Yu Kuai <yukuai1@...weicloud.com>
Cc: axboe@...nel.dk, song@...nel.org, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-raid@...r.kernel.org,
 yi.zhang@...wei.com, yangerkun@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH 3/4] md: fix is_mddev_idle()

Hi,

在 2025/04/16 17:44, Xiao Ni 写道:
> On Wed, Apr 16, 2025 at 5:29 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>
>> Hi,
>>
>> 在 2025/04/16 15:42, Yu Kuai 写道:
>>> Hi,
>>>
>>> 在 2025/04/16 14:20, Xiao Ni 写道:
>>>>> +static bool is_rdev_idle(struct md_rdev *rdev, bool init)
>>>>> +{
>>>>> +    unsigned long last_events = rdev->last_events;
>>>>> +
>>>>> +    if (!bdev_is_partition(rdev->bdev))
>>>>> +        return true;
>>>>
>>>>
>>>> For md array, I think is_rdev_idle is not useful. Because
>>>> mddev->last_events must be increased while upper ios come in and idle
>>>> will be set to false. For dm array, mddev->last_events can't work. So
>>>> is_rdev_idle is for dm array. If member disk is one partition,
>>>> is_rdev_idle alwasy returns true, and is_mddev_idle always return
>>>> true. It's a bug here. Do we need to check bdev_is_partition here?
>>>
>>> is_rdev_idle() is not used for current array, for example:
>>>
>>> sda1 is used for array md0, and user doesn't issue IO to md0, while
>>> user issues IO to sda2. In this case, is_mddev_idle() still fail for
>>> array md0 because is_rdev_idle() fail.
> 
> Thanks very much for the explanation. It makes sense :)
> 
>>
>> Perhaps the name is_rdev_holder_idle() is better.
> 
> Your suggestion is better. And it's better to add some comments before
> this function.
> 
> But how about dm-raid? Can this patch work for dm-raid?

is_rdev_holder_idle() can work for dm-raid, however, the part to
check if normal IO is inflight or completed can't work for dm-raid,
currently there is no way to grab dm gendisk from mddev. However, I
think there won't be regression since the old buggy is_mddev_idle()
almost always return false.

Thanks,
Kuai

> 
> Regards
> Xiao
> 
>>
>> Thanks,
>> Kuai
>>
>>>
>>> This is just inherited from the old behaviour.
>>>
>>> Thanks,
>>> Kuai
>>>
>>>>
>>>> Best Regards
>>>>
>>>> Xiao
>>>
>>> .
>>>
>>
> 
> 
> .
>