[PATCH 0/2] Improvements for barrier-test scheduler

[PATCH 0/2] Improvements for barrier-test scheduler

[Jan Kara]

  Hello Chris,

  I was hunting one ext3/4 barrier related bug and so I used your barrier-test
IO scheduler. It was quite helpful so thank you for it. For my purposes, I
had to tweak it a bit to trigger my problem. I'm not sure if you keep the
scheduler somewhere but if yes, I hope my patches could be of use.

Patch 1 implements raw support for handling reads - we don't have to flush
our delayed writes everytime a read comes. It is enough to do so when some
delayed write overlaps with a read. Scanning the list of delayed writes is slow
but who really cares for barrier-test :)

Patch 2 implements another criteria for reordering requests and triggering
reboot. Filesystem can flag certain bios as special. These bios will go
directly to the backing storage and when they complete, reboot is triggered
if there are enough pending writes. Filesystem can use the flag when writing
commit blocks, superblocks or similarly sensitive metadata to verify whether
barriers are issued in all the necessary places.

								Honza

[PATCH 1/2] barrier-test: Do not flush delayed queue for reads

[Jan Kara]

Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/barrier-test-iosched.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/block/barrier-test-iosched.c b/block/barrier-test-iosched.c
index f641537..8c30461 100644
--- a/block/barrier-test-iosched.c
+++ b/block/barrier-test-iosched.c
@@ -78,8 +78,6 @@ struct barrier_test_data {
 	int total_barriers;
 };
 
-typedef void (rq_end_io_fn)(struct request *, int);
-
 /*
  * wb_requests are ones that we are pretending are done.  We
  * just return completion right away to the poor user.
@@ -429,7 +427,23 @@ static void barrier_test_add_request(struct request_queue *q, struct request *rq
 	 * reads only happen after all the writeback io is complete.
 	 */
 	if (!(rq->cmd_flags & REQ_WRITE)) {
-		if (atomic_read(&nd->pending_rqs)) {
+		struct request *wrq;
+		bool overlap = false;
+
+		list_for_each_entry(wrq, &nd->delayed_requests, queuelist) {
+			if (blk_rq_pos(wrq) <= blk_rq_pos(rq) &&
+			    blk_rq_pos(wrq) + blk_rq_sectors(wrq) >
+							blk_rq_pos(rq)) {
+				overlap = true;
+				break;
+			} else if (blk_rq_pos(wrq) >= blk_rq_pos(rq) &&
+				   blk_rq_pos(wrq) <
+					blk_rq_pos(rq) + blk_rq_sectors(rq)) {
+				overlap = true;
+				break;
+			}
+		}
+		if (overlap) {
 			queue_all_delayed(nd, 0);
 			list_add_tail(&rq->queuelist, &nd->reads_waiting_flush);
 		} else {
-- 
1.7.1

[PATCH 2/2] barrier-test: Allow filesystem to specify special test request

[Jan Kara]

Create new request flag by which a filesystem can say that this block is
special and it's reordering is interesting for causing breakage. So we submit
the block immediately and possibly crash just after the IO is finished.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/barrier-test-iosched.c |   41 +++++++++++++++++++++++++++++++----------
 include/linux/blk_types.h    |    6 +++++-
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/block/barrier-test-iosched.c b/block/barrier-test-iosched.c
index 8c30461..1d66097 100644
--- a/block/barrier-test-iosched.c
+++ b/block/barrier-test-iosched.c
@@ -74,7 +74,9 @@ struct barrier_test_data {
 	/* this is for waiting until prending_rqs is zero */
 	wait_queue_head_t pending_rq_wait;
 
-	int random_reboot;
+	int random_flush_reboot;
+	int random_rq_reboot;
+
 	int total_barriers;
 };
 
@@ -144,7 +146,13 @@ static void barrier_test_end_io_fn(struct request *rq, int err)
 
 	q = rq->q;
 	nd = q->elevator->elevator_data;
-
+	if (rq->cmd_flags & REQ_BARRIER_TEST &&
+	    atomic_read(&nd->pending_rqs) > 256 &&
+	    nd->random_rq_reboot) {
+		printk(KERN_CRIT "doing random crash after request "
+			"completion\n");
+		emergency_restart();
+	}
 	list_add(&rq->queuelist, &nd->spare_requests);
 
 	/* if there are no more pending requests, we can
@@ -200,7 +208,8 @@ static int can_delay_request(struct request *rq)
 {
 	if (!(rq->cmd_flags & REQ_WRITE))
 		return 0;
-	if (rq->cmd_flags & (REQ_DISCARD | REQ_FLUSH | REQ_FUA))
+	if (rq->cmd_flags &
+	    (REQ_DISCARD | REQ_FLUSH | REQ_FUA))
 		return 0;
 	if (rq->bio != rq->biotail)
 		return 0;
@@ -360,7 +369,7 @@ static void queue_all_delayed(struct barrier_test_data *nd, int for_barrier)
 		 * some
 		 */
 		if (for_barrier && orig_delayed_nr > 256 &&
-		    nr == 256 && nd->random_reboot) {
+		    nr == 256 && nd->random_flush_reboot) {
 			printk(KERN_CRIT "doing random crash before barrier submit\n");
 			emergency_restart();
 		}
@@ -486,8 +495,16 @@ static void barrier_test_add_request(struct request_queue *q, struct request *rq
 	 */
 	list_add_tail(&rq->queuelist, &nd->writeback_requests);
 
-	/* add our copy to the delayed_requests list */
-	list_add_tail(&copy->queuelist, &nd->delayed_requests);
+	if (!(copy->cmd_flags & REQ_BARRIER_TEST)) {
+		/* add our copy to the delayed_requests list */
+		list_add_tail(&copy->queuelist, &nd->delayed_requests);
+	} else {
+		/*
+		 * add our copy directly to dispatch list. We couldn't do it
+		 * with original request since we need special end_io function
+		 */
+		list_add_tail(&copy->queuelist, &nd->queue);
+	}
 
 	/* if we're depleting our list of spares, start the writeback IO */
 	if (atomic_read(&nd->pending_rqs) > (NR_SPARE_REQUESTS / 2))
@@ -560,7 +577,8 @@ static void *barrier_test_init_queue(struct request_queue *q)
 	init_waitqueue_head(&nd->pending_rq_wait);
 	atomic_set(&nd->pending_rqs, 0);
 	bio_list_init(&nd->spare_bios);
-	nd->random_reboot = 0;
+	nd->random_flush_reboot = 0;
+	nd->random_rq_reboot = 0;
 	nd->total_barriers = 0;
 
 	for (i = 0; i < NR_SPARE_REQUESTS; i++) {
@@ -632,7 +650,8 @@ static ssize_t __FUNC(struct elevator_queue *e, char *page)		\
 		__data = jiffies_to_msecs(__data);			\
 	return barrier_test_var_show(__data, (page));			\
 }
-SHOW_FUNCTION(barrier_test_random_reboot_show, dd->random_reboot, 0);
+SHOW_FUNCTION(barrier_test_random_flush_reboot_show, dd->random_flush_reboot, 0);
+SHOW_FUNCTION(barrier_test_random_rq_reboot_show, dd->random_rq_reboot, 0);
 #undef SHOW_FUNCTION
 
 #define STORE_FUNCTION(__FUNC, __PTR, MIN, MAX, __CONV)			\
@@ -651,7 +670,8 @@ static ssize_t __FUNC(struct elevator_queue *e, const char *page, size_t count)
 		*(__PTR) = __data;					\
 	return ret;							\
 }
-STORE_FUNCTION(barrier_test_random_reboot_store, &dd->random_reboot, 0, 1, 0);
+STORE_FUNCTION(barrier_test_random_flush_reboot_store, &dd->random_flush_reboot, 0, 1, 0);
+STORE_FUNCTION(barrier_test_random_rq_reboot_store, &dd->random_rq_reboot, 0, 1, 0);
 #undef STORE_FUNCTION
 
 #define BT_ATTR(name) \
@@ -659,7 +679,8 @@ STORE_FUNCTION(barrier_test_random_reboot_store, &dd->random_reboot, 0, 1, 0);
 				      barrier_test_##name##_store)
 
 static struct elv_fs_entry barrier_test_attrs[] = {
-	BT_ATTR(random_reboot),
+	BT_ATTR(random_flush_reboot),
+	BT_ATTR(random_rq_reboot),
 	__ATTR_NULL
 };
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 4053cbd..1e94ebc 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -128,6 +128,8 @@ enum rq_flag_bits {
 	__REQ_NOIDLE,		/* don't anticipate more IO after this one */
 	__REQ_FUA,		/* forced unit access */
 	__REQ_FLUSH,		/* request for cache flush */
+	__REQ_BARRIER_TEST,	/* request whose reordering is interesting
+				 * for barrier test */
 
 	/* bio only flags */
 	__REQ_RAHEAD,		/* read ahead, can fail anytime */
@@ -162,12 +164,14 @@ enum rq_flag_bits {
 #define REQ_PRIO		(1 << __REQ_PRIO)
 #define REQ_DISCARD		(1 << __REQ_DISCARD)
 #define REQ_NOIDLE		(1 << __REQ_NOIDLE)
+#define REQ_BARRIER_TEST	(1 << __REQ_BARRIER_TEST)
 
 #define REQ_FAILFAST_MASK \
 	(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
 #define REQ_COMMON_MASK \
 	(REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_PRIO | \
-	 REQ_DISCARD | REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE)
+	 REQ_DISCARD | REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE | \
+	 REQ_BARRIER_TEST)
 #define REQ_CLONE_MASK		REQ_COMMON_MASK
 
 #define REQ_RAHEAD		(1 << __REQ_RAHEAD)
-- 
1.7.1

Re: [PATCH 0/2] Improvements for barrier-test scheduler

[Chris Mason]

On Wed, Jan 11, 2012 at 01:44:05AM +0100, Jan Kara wrote:
> 
>   Hello Chris,
> 
>   I was hunting one ext3/4 barrier related bug and so I used your barrier-test
> IO scheduler. It was quite helpful so thank you for it. For my purposes, I
> had to tweak it a bit to trigger my problem. I'm not sure if you keep the
> scheduler somewhere but if yes, I hope my patches could be of use.
> 
> Patch 1 implements raw support for handling reads - we don't have to flush
> our delayed writes everytime a read comes. It is enough to do so when some
> delayed write overlaps with a read. Scanning the list of delayed writes is slow
> but who really cares for barrier-test :)

Perfect ;)

> 
> Patch 2 implements another criteria for reordering requests and triggering
> reboot. Filesystem can flag certain bios as special. These bios will go
> directly to the backing storage and when they complete, reboot is triggered
> if there are enough pending writes. Filesystem can use the flag when writing
> commit blocks, superblocks or similarly sensitive metadata to verify whether
> barriers are issued in all the necessary places.

Great idea, I'll fold this in.

Jens, is this something you want in 3.3, or would you like to let it
cook for another release cycle?

-chris

Re: [PATCH 0/2] Improvements for barrier-test scheduler

[Jens Axboe]

On 01/11/2012 02:22 AM, Chris Mason wrote:
> Jens, is this something you want in 3.3, or would you like to let it
> cook for another release cycle?

Since it's a test/debug aid and it's already proven itself useful, I
don't see any reason to push it out another cycle.

-- 
Jens Axboe