[DRBD-cvs] svn commit by lars - r2919 - branches/drbd-8.0/drbd - paranoia code in our bi_end_io handlers to shield again

[drbd-cvs at lists.linbit.com]

Author: lars
Date: 2007-06-18 17:22:12 +0200 (Mon, 18 Jun 2007)
New Revision: 2919

Modified:
   branches/drbd-8.0/drbd/drbd_actlog.c
   branches/drbd-8.0/drbd/drbd_bitmap.c
   branches/drbd-8.0/drbd/drbd_worker.c
Log:
paranoia code in our bi_end_io handlers
to shield agains strange lower level drivers which uppon failure to serve
a request just clear the UPTODATE flag, but report no error!

this is known to have happend for READA on md raid5,
but may happen elsewhere, too.

CAUTION:
 this could break things in case there exist cases
 where it is no error when the UPTODATE flag is cleared.
 (I don't think there are...
  in my understanding there at least should not be).

[btw: already committed on trunk]


Modified: branches/drbd-8.0/drbd/drbd_actlog.c
===================================================================
--- branches/drbd-8.0/drbd/drbd_actlog.c	2007-06-18 14:14:04 UTC (rev 2918)
+++ branches/drbd-8.0/drbd/drbd_actlog.c	2007-06-18 15:22:12 UTC (rev 2919)
@@ -527,8 +527,16 @@
 	struct drbd_atodb_wait *wc = bio->bi_private;
 	struct Drbd_Conf *mdev=wc->mdev;
 	struct page *page;
+	int uptodate = bio_flagged(bio,BIO_UPTODATE);
 
 	if (bio->bi_size) return 1;
+	if (!error && !uptodate) {
+		/* strange behaviour of some lower level drivers...
+		 * fail the request by clearing the uptodate flag,
+		 * but do not return any error?!
+		 * do we want to WARN() on this? */
+		error = -EIO;
+	}
 
 	drbd_chk_io_error(mdev,error,TRUE);
 	if(error && wc->error == 0) wc->error=error;

Modified: branches/drbd-8.0/drbd/drbd_bitmap.c
===================================================================
--- branches/drbd-8.0/drbd/drbd_bitmap.c	2007-06-18 14:14:04 UTC (rev 2918)
+++ branches/drbd-8.0/drbd/drbd_bitmap.c	2007-06-18 15:22:12 UTC (rev 2919)
@@ -629,9 +629,17 @@
 int drbd_bm_async_io_complete(struct bio *bio, unsigned int bytes_done, int error)
 {
 	struct drbd_bitmap *b = bio->bi_private;
+	int uptodate = bio_flagged(bio,BIO_UPTODATE);
 
 	if (bio->bi_size)
 		return 1;
+	if (!error && !uptodate) {
+		/* strange behaviour of some lower level drivers...
+		 * fail the request by clearing the uptodate flag,
+		 * but do not return any error?!
+		 * do we want to WARN() on this? */
+		error = -EIO;
+	}
 
 	if (error) {
 		/* doh. what now?

Modified: branches/drbd-8.0/drbd/drbd_worker.c
===================================================================
--- branches/drbd-8.0/drbd/drbd_worker.c	2007-06-18 14:14:04 UTC (rev 2918)
+++ branches/drbd-8.0/drbd/drbd_worker.c	2007-06-18 15:22:12 UTC (rev 2919)
@@ -42,8 +42,16 @@
 #include "drbd_int.h"
 #include "drbd_req.h"
 
-/* I choose to have all block layer end_io handlers defined here.
+/* defined here:
+   drbd_md_io_complete
+   drbd_endio_write_sec
+   drbd_endio_read_sec
+   drbd_endio_pri
 
+ * more endio handlers:
+   atodb_endio in drbd_actlog.c
+   drbd_bm_async_io_complete in drbd_bitmap.c
+
  * For all these callbacks, note the follwing:
  * The callbacks will be called in irq context by the IDE drivers,
  * and in Softirqs/Tasklets/BH context by the SCSI drivers.
@@ -57,6 +65,8 @@
 int drbd_md_io_complete(struct bio *bio, unsigned int bytes_done, int error)
 {
 	if (bio->bi_size) return 1;
+	/* error parameter ignored:
+	 * drbd_md_sync_page_io explicitly tests bio_uptodate(bio); */
 
 	complete((struct completion*)bio->bi_private);
 	return 0;
@@ -70,6 +80,7 @@
 	unsigned long flags=0;
 	struct Tl_epoch_entry *e=NULL;
 	struct Drbd_Conf* mdev;
+	int uptodate = bio_flagged(bio, BIO_UPTODATE);
 
 	e = bio->bi_private;
 	mdev = e->mdev;
@@ -78,6 +89,13 @@
 	 * we are only interested when the whole bio is finished, therefore
 	 * return as long as bio->bio_size is positive.  */
 	if (bio->bi_size) return 1;
+	if (!error && !uptodate) {
+		/* strange behaviour of some lower level drivers...
+		 * fail the request by clearing the uptodate flag,
+		 * but do not return any error?!
+		 * do we want to WARN() on this? */
+		error = -EIO;
+	}
 
 	D_ASSERT(e->block_id != ID_VACANT);
 
@@ -110,12 +128,20 @@
 	int do_wake;
 	int is_syncer_req;
 	int do_al_complete_io;
+	int uptodate = bio_flagged(bio, BIO_UPTODATE);
 
 	e = bio->bi_private;
 	mdev = e->mdev;
 
 	// see above
 	if (bio->bi_size) return 1;
+	if (!error && !uptodate) {
+		/* strange behaviour of some lower level drivers...
+		 * fail the request by clearing the uptodate flag,
+		 * but do not return any error?!
+		 * do we want to WARN() on this? */
+		error = -EIO;
+	}
 
 	D_ASSERT(e->block_id != ID_VACANT);
 
@@ -172,9 +198,17 @@
 	drbd_request_t *req=bio->bi_private;
 	drbd_dev *mdev = req->mdev;
 	drbd_req_event_t what;
+	int uptodate = bio_flagged(bio, BIO_UPTODATE);
 
 	// see above
 	if (bio->bi_size) return 1;
+	if (!error && !uptodate) {
+		/* strange behaviour of some lower level drivers...
+		 * fail the request by clearing the uptodate flag,
+		 * but do not return any error?!
+		 * do we want to WARN() on this? */
+		error = -EIO;
+	}
 
 	/* to avoid recursion in _req_mod */
 	what = error