block: remove duplicate or unused barrier/discard error paths

* Because barrier mode can be changed dynamically, whether barrier is supported or not can be determined only when actually issuing the barrier and there is no point in checking it earlier. Drop barrier support check in generic_make_request() and __make_request(), and update comment around the support check in blk_do_ordered(). * There is no reason to check discard support in both generic_make_request() and __make_request(). Drop the check in __make_request(). While at it, move error action block to the end of the function and add unlikely() to q existence test. * Barrier request, be it empty or not, is never passed to low level driver and thus it's meaningless to try to copy back req->sector to bio->bi_sector on error. In addition, the notion of failed sector doesn't make any sense for empty barrier to begin with. Drop the code block from __end_that_request_first(). Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <jens.axboe@oracle.com>

block: remove duplicate or unused barrier/discard error paths
* Because barrier mode can be changed dynamically, whether barrier is supported or not can be determined only when actually issuing the barrier and there is no point in checking it earlier. Drop barrier support check in generic_make_request() and __make_request(), and update comment around the support check in blk_do_ordered(). * There is no reason to check discard support in both generic_make_request() and __make_request(). Drop the check in __make_request(). While at it, move error action block to the end of the function and add unlikely() to q existence test. * Barrier request, be it empty or not, is never passed to low level driver and thus it's meaningless to try to copy back req->sector to bio->bi_sector on error. In addition, the notion of failed sector doesn't make any sense for empty barrier to begin with. Drop the code block from __end_that_request_first(). Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
Tejun Heo · Jens Axboe
1 parent 313e42999d
Showing 2 changed files with 13 additions and 35 deletions Side-by-side Diff
block/blk-barrier.c
block/blk-core.c
@@ -216,8 +216,8 @@
 			return 1;
 		} else {
 			/*
-			 * This can happen when the queue switches to
-			 * ORDERED_NONE while this request is on it.
+			 * Queue ordering not supported.  Terminate
+			 * with prejudice.
 			 */
 			elv_dequeue_request(q, rq);
 			if (__blk_end_request(rq, -EOPNOTSUPP,
@@ -1139,7 +1139,7 @@
 static int __make_request(struct request_queue *q, struct bio *bio)
 {
 	struct request *req;
-	int el_ret, nr_sectors, barrier, discard, err;
+	int el_ret, nr_sectors;
 	const unsigned short prio = bio_prio(bio);
 	const int sync = bio_sync(bio);
 	int rw_flags;
  
@@ -1153,22 +1153,9 @@
 	 */
 	blk_queue_bounce(q, &bio);
  
-	barrier = bio_barrier(bio);
-	if (unlikely(barrier) && bio_has_data(bio) &&
-	    (q->next_ordered == QUEUE_ORDERED_NONE)) {
-		err = -EOPNOTSUPP;
-		goto end_io;
-	}
-
-	discard = bio_discard(bio);
-	if (unlikely(discard) && !q->prepare_discard_fn) {
-		err = -EOPNOTSUPP;
-		goto end_io;
-	}
-
 	spin_lock_irq(q->queue_lock);
  
-	if (unlikely(barrier) || elv_queue_empty(q))
+	if (unlikely(bio_barrier(bio)) || elv_queue_empty(q))
 		goto get_rq;
  
 	el_ret = elv_merge(q, &req, bio);
@@ -1262,10 +1249,6 @@
 		__generic_unplug_device(q);
 	spin_unlock_irq(q->queue_lock);
 	return 0;
-
-end_io:
-	bio_endio(bio, err);
-	return 0;
 }
  
 /*
  
@@ -1418,15 +1401,13 @@
 		char b[BDEVNAME_SIZE];
  
 		q = bdev_get_queue(bio->bi_bdev);
-		if (!q) {
+		if (unlikely(!q)) {
 			printk(KERN_ERR
 			       "generic_make_request: Trying to access "
 				"nonexistent block-device %s (%Lu)\n",
 				bdevname(bio->bi_bdev, b),
 				(long long) bio->bi_sector);
-end_io:
-			bio_endio(bio, err);
-			break;
+			goto end_io;
 		}
  
 		if (unlikely(nr_sectors > q->max_hw_sectors)) {
  
@@ -1463,14 +1444,19 @@
  
 		if (bio_check_eod(bio, nr_sectors))
 			goto end_io;
-		if ((bio_empty_barrier(bio) && !q->prepare_flush_fn) ||
-		    (bio_discard(bio) && !q->prepare_discard_fn)) {
+
+		if (bio_discard(bio) && !q->prepare_discard_fn) {
 			err = -EOPNOTSUPP;
 			goto end_io;
 		}
  
 		ret = q->make_request_fn(q, bio);
 	} while (ret);
+
+	return;
+
+end_io:
+	bio_endio(bio, err);
 }
  
 /*
@@ -1719,14 +1705,6 @@
 	total_bytes = bio_nbytes = 0;
 	while ((bio = req->bio) != NULL) {
 		int nbytes;
-
-		/*
-		 * For an empty barrier request, the low level driver must
-		 * store a potential error location in ->sector. We pass
-		 * that back up in ->bi_sector.
-		 */
-		if (blk_empty_barrier(req))
-			bio->bi_sector = req->sector;
  
 		if (nr_bytes >= bio->bi_size) {
 			req->bio = bio->bi_next;
...	...	@@ -216,8 +216,8 @@
216	216	return 1;
217	217	} else {
218	218	/*
219		- * This can happen when the queue switches to
220		- * ORDERED_NONE while this request is on it.
	219	+ * Queue ordering not supported. Terminate
	220	+ * with prejudice.
221	221	*/
222	222	elv_dequeue_request(q, rq);
223	223	if (__blk_end_request(rq, -EOPNOTSUPP,
...	...	@@ -1139,7 +1139,7 @@
1139	1139	static int __make_request(struct request_queue q, struct bio bio)
1140	1140	{
1141	1141	struct request *req;
1142		- int el_ret, nr_sectors, barrier, discard, err;
	1142	+ int el_ret, nr_sectors;
1143	1143	const unsigned short prio = bio_prio(bio);
1144	1144	const int sync = bio_sync(bio);
1145	1145	int rw_flags;
1146	1146
...	...	@@ -1153,22 +1153,9 @@
1153	1153	*/
1154	1154	blk_queue_bounce(q, &bio);
1155	1155
1156		- barrier = bio_barrier(bio);
1157		- if (unlikely(barrier) && bio_has_data(bio) &&
1158		- (q->next_ordered == QUEUE_ORDERED_NONE)) {
1159		- err = -EOPNOTSUPP;
1160		- goto end_io;
1161		- }
1162		-
1163		- discard = bio_discard(bio);
1164		- if (unlikely(discard) && !q->prepare_discard_fn) {
1165		- err = -EOPNOTSUPP;
1166		- goto end_io;
1167		- }
1168		-
1169	1156	spin_lock_irq(q->queue_lock);
1170	1157
1171		- if (unlikely(barrier) \|\| elv_queue_empty(q))
	1158	+ if (unlikely(bio_barrier(bio)) \|\| elv_queue_empty(q))
1172	1159	goto get_rq;
1173	1160
1174	1161	el_ret = elv_merge(q, &req, bio);
...	...	@@ -1262,10 +1249,6 @@
1262	1249	__generic_unplug_device(q);
1263	1250	spin_unlock_irq(q->queue_lock);
1264	1251	return 0;
1265		-
1266		-end_io:
1267		- bio_endio(bio, err);
1268		- return 0;
1269	1252	}
1270	1253
1271	1254	/*
1272	1255
...	...	@@ -1418,15 +1401,13 @@
1418	1401	char b[BDEVNAME_SIZE];
1419	1402
1420	1403	q = bdev_get_queue(bio->bi_bdev);
1421		- if (!q) {
	1404	+ if (unlikely(!q)) {
1422	1405	printk(KERN_ERR
1423	1406	"generic_make_request: Trying to access "
1424	1407	"nonexistent block-device %s (%Lu)\n",
1425	1408	bdevname(bio->bi_bdev, b),
1426	1409	(long long) bio->bi_sector);
1427		-end_io:
1428		- bio_endio(bio, err);
1429		- break;
	1410	+ goto end_io;
1430	1411	}
1431	1412
1432	1413	if (unlikely(nr_sectors > q->max_hw_sectors)) {
1433	1414
...	...	@@ -1463,14 +1444,19 @@
1463	1444
1464	1445	if (bio_check_eod(bio, nr_sectors))
1465	1446	goto end_io;
1466		- if ((bio_empty_barrier(bio) && !q->prepare_flush_fn) \|\|
1467		- (bio_discard(bio) && !q->prepare_discard_fn)) {
	1447	+
	1448	+ if (bio_discard(bio) && !q->prepare_discard_fn) {
1468	1449	err = -EOPNOTSUPP;
1469	1450	goto end_io;
1470	1451	}
1471	1452
1472	1453	ret = q->make_request_fn(q, bio);
1473	1454	} while (ret);
	1455	+
	1456	+ return;
	1457	+
	1458	+end_io:
	1459	+ bio_endio(bio, err);
1474	1460	}
1475	1461
1476	1462	/*
...	...	@@ -1719,14 +1705,6 @@
1719	1705	total_bytes = bio_nbytes = 0;
1720	1706	while ((bio = req->bio) != NULL) {
1721	1707	int nbytes;
1722		-
1723		- /*
1724		- * For an empty barrier request, the low level driver must
1725		- * store a potential error location in ->sector. We pass
1726		- * that back up in ->bi_sector.
1727		- */
1728		- if (blk_empty_barrier(req))
1729		- bio->bi_sector = req->sector;
1730	1708
1731	1709	if (nr_bytes >= bio->bi_size) {
1732	1710	req->bio = bio->bi_next;