Re: [External] Re: [PATCH] blk-throtl: Introduce sync and async queues for blk-throtl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2022/12/20 上午5:22, Tejun Heo 写道:
Hello,

This looks generally fine to me. Some nits below.

+static inline struct bio *throtl_qnode_bio_peek(struct throtl_qnode *qn)
+{
+	struct bio *bio1, *bio2;
+
+	/* qn for read ios */
+	if (qn->dispatch_sync_cnt == UINT_MAX)
+		return bio_list_peek(&qn->bios[SYNC]);
+
+	/* qn for write ios */
+	bio1 = bio_list_peek(&qn->bios[SYNC]);
+	bio2 = bio_list_peek(&qn->bios[ASYNC]);
+
+	if (bio1 && bio2) {
+		if (qn->dispatch_sync_cnt == THROTL_SYNC_FACTOR)
+			return bio2;
+		return bio1;
+	}
+
+	return bio1 ?: bio2;
+}

Wouldn't it be simpler to write:

         if (qn->dispatch_sync_count < THROTL_SYNC_FACTOR)
                 return bio1 ?: bio2;
         else
                 return bio2 ?: bio1;

+/**
+ * throtl_qnode_bio_pop: pop a bio from a qnode
+ * @qn: the qnode to pop a bio from
+ *
+ * For read io qn, just pop bio from sync queu and return.
+ * For write io qn, the target queue to pop was determined by the dispatch_sync_cnt.
+ * Try to pop bio from target queue, fetch the bio and return when it is not empty.
+ * If the target queue empty, pop bio from other queue instead.
+ */
+static inline struct bio *throtl_qnode_bio_pop(struct throtl_qnode *qn)
+{
+	struct bio *bio;
+
+	/* qn for read ios */
+	if (qn->dispatch_sync_cnt == UINT_MAX)
+		return bio_list_pop(&qn->bios[SYNC]);
+
+	/* try to dispatch sync io */
+	if (qn->dispatch_sync_cnt < THROTL_SYNC_FACTOR) {
+		bio = bio_list_pop(&qn->bios[SYNC]);
+		if (bio) {
+			qn->dispatch_sync_cnt++;
+			return bio;
+		}
+		bio = bio_list_pop(&qn->bios[ASYNC]);
+		qn->dispatch_sync_cnt = 0;
+		return bio;
+	}
+
+	/* try to dispatch async io */
+	bio = bio_list_pop(&qn->bios[ASYNC]);
+	if (bio) {
+		qn->dispatch_sync_cnt = 0;
+		return bio;
+	}
+	bio = bio_list_pop(&qn->bios[SYNC]);
+
+	return bio;
+}

This also seems like it can be simplified a bit.

Thanks.


Indeed, I will make it more clear and send the v2.

Thanks.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux