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
