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:

quoted

+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,

quoted

+        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

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel