Skip to contentSkip to conversation

libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

PATCH In review Jens Axboe 2 participants 3 messages May 20, 2009 libata

lore
Share & tools
lore

Jens Axboe

author
#0 lore raw

PATCH 1/1 libata: get rid of ATA_MAX_QUEUE loop in ata_qc_complete_multiple()

Hi,

We very rarely (if ever) complete more than one command in the sactive mask at the time, even for extremely high IO rates. So looping over the entire range of possible tags is pointless, instead use __ffs() to just find the completed tags directly.

Signed-off-by Jens Axboe <[email protected]>
---
 drivers/ata/libata-core.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
drivers/ata/libata-core.c +9 -6
index 17c5d48..8de0081 100644
@@ -5027,9 +5027,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
  */
 int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 {
+	unsigned int i = 0;
 	int nr_done = 0;
 	u32 done_mask;
-	int i;
 
 	done_mask = ap->qc_active ^ qc_active;
 

@@ -5039,16 +5039,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < ATA_MAX_QUEUE; i++) {
+	while (done_mask) {
 		struct ata_queued_cmd *qc;
+		unsigned int next = __ffs(done_mask);
 
-		if (!(done_mask & (1 << i)))
-			continue;
-
-		if ((qc = ata_qc_from_tag(ap, i))) {
+		qc = ata_qc_from_tag(ap, i + next);
+		if (qc) {
 			ata_qc_complete(qc);
 			nr_done++;
 		}
+		if (++next >= ATA_MAX_QUEUE)
+			break;
+		i += next;
+		done_mask >>= next;
 	}
 
 	return nr_done;
Signature · 5 lines
1.6.3.rc0.1.gf800


-- 
Jens Axboe
#1 lore raw

Hello, Jens.

Quoted 10 lines · Jens Axboe
drivers/ata/libata-core.c +2 -2
@@ -5039,16 +5039,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < ATA_MAX_QUEUE; i++) {
+	while (done_mask) {
 		struct ata_queued_cmd *qc;
+		unsigned int next = __ffs(done_mask);
 
-		if (!(done_mask & (1 << i)))

drivers/ata/libata-core.c int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active) · line 5052

Quoted 10 lines
drivers/ata/libata-core.c +4 -3
-			continue;
-
-		if ((qc = ata_qc_from_tag(ap, i))) {
+		qc = ata_qc_from_tag(ap, i + next);
+		if (qc) {
 			ata_qc_complete(qc);
 			nr_done++;
 		}
+		if (++next >= ATA_MAX_QUEUE)
+			break;

Given that __ffs doesn't care how far the first bit is on most archs (even the generic one), I think it would be simpler and more efficient to simply do done_mask &= ~(1 << i) and loop without adding to index and shifting.

Thanks.

Signature · 1 line
tejun
author
#2 lore raw
Quoted 12 lines · Tejun Heo

Hello, Jens.

Quoted 9 lines · Jens Axboe
drivers/ata/libata-core.c +2 -1
@@ -5039,16 +5039,19 @@ int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < ATA_MAX_QUEUE; i++) {
+	while (done_mask) {
 		struct ata_queued_cmd *qc;
+		unsigned int next = __ffs(done_mask);
Quoted 16 lines

drivers/ata/libata-core.c int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active) · line 5052

Quoted 11 lines
drivers/ata/libata-core.c +4 -4
-		if (!(done_mask & (1 << i)))
-			continue;
-
-		if ((qc = ata_qc_from_tag(ap, i))) {
+		qc = ata_qc_from_tag(ap, i + next);
+		if (qc) {
 			ata_qc_complete(qc);
 			nr_done++;
 		}
+		if (++next >= ATA_MAX_QUEUE)
+			break;

Given that __ffs doesn't care how far the first bit is on most archs (even the generic one), I think it would be simpler and more efficient to simply do done_mask &= ~(1 << i) and loop without adding to index and shifting.

It would be easier to read as well, agree. I'll repost it in 2 secs.

Signature · 1 line
Jens Axboe