Re: [PATCH v3 3/6] badblocks: improvement badblocks_set() for multiple ranges handling

On 9/27/21 3:30 PM, Geliang Tang wrote:

On 9/14/21 00:36, Coly Li wrote:

[snipped]

quoted

+ * 2.1.1) If S and E are both acked or unacked range, the setting range S can
+ *    be merged into existing bad range E. The result is,
+ *        +-------------+
+ *        |      S      |
+ *        +-------------+
+ * 2.1.2) If S is uncked setting and E is acked, the setting will be dinied, and

uncked -> unacked
dinied?

You are correct, it should be unacked.

quoted

+ *    the result is,
+ *        +-------------+
+ *        |      E      |
+ *        +-------------+
+ * 2.1.3) If S is acked setting and E is unacked, range S can overwirte on E.
+ *    An extra slot from the bad blocks table will be allocated for S, and head
+ *    of E will move to end of the inserted range E. The result is,
+ *        +--------+----+
+ *        |    S   | E  |
+ *        +--------+----+
+ * 2.2) The setting range size == already set range size
+ * 2.2.1) If S and E are both acked or unacked range, the setting range S can
+ *    be merged into existing bad range E. The result is,
+ *        +-------------+
+ *        |      S      |
+ *        +-------------+
+ * 2.2.2) If S is uncked setting and E is acked, the setting will be dinied, and

uncked -> unacked

Yes, thanks for pointing out the typo. I will fix them in next version.

[snipped]

quoted

+/* Do exact work to set bad block range into the bad block table */
+static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+              int acknowledged)
+{
+    u64 *p;
+    struct badblocks_context bad;
+    int prev = -1, hint = -1;
+    int len = 0, added = 0;
+    int retried = 0, space_desired = 0;
+    int rv = 0;
+    unsigned long flags;

orig_start and orig_len are used in _badblocks_set() only, we can drop them from struct badblocks_context, declare two local variables instead:

        sector_t orig_start;
        int orig_len;

It's fair, let me change it in next version.

[snipped]

Thanks for your review!

Coly Li