Re: [PATCH v2 1/2] md raid0/linear: Introduce new array state 'broken'

["Guilherme G. Piccoli" <gpiccoli@canonical.com>]

On 22/08/2019 05:49, Guoqing Jiang wrote:
> Hi,

Hi Guoqing, thanks for the review!


> 
> On 8/16/19 3:40 PM, Guilherme G. Piccoli wrote:
>> +static bool linear_is_missing_dev(struct mddev *mddev)
>> +{
>> +    struct md_rdev *rdev;
>> +    static int already_missing;
>> +    int def_disks, work_disks = 0;
>> +
>> +    def_disks = mddev->raid_disks;
>> +    rdev_for_each(rdev, mddev)
>> +        if (rdev->bdev->bd_disk->flags & GENHD_FL_UP)
>> +            work_disks++;
>> +
>> +    if (unlikely(def_disks - work_disks)) {
> 
> If my understanding is correct, after enter the branch,
> 
>> +        if (already_missing < (def_disks - work_disks)) {
> 
> the above is always true since already_missing should be '0', right?
> If so, maybe the above checking is pointless.

The variable 'already_missing' is static, so indeed it starts with 0.
When there are missing disks, we'll enter the 'if(def_disks -
work_disks)' and indeed print the message. But notice we'll set
'already_missing = def_disks - work_disks', so we won't enter the if and
print the message anymore _unless_ a new member is removed and
'already_missing' gets unbalanced with regards to 'def_disks - work_disks'.

The idea behind it is to show a single message whenever a new member is
removed. The use of a static variable allows us to do that.

Nevertheless, this path will be dropped in the V3 that is ready to be sent.
Cheers,


Guilherme