Re: [Drbd-dev] [fix] drbd uses wrong API for struct bio

[Lars Marowsky-Bree <lmb@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1192 bytes --]

On 2005-01-23T17:16:33, Lars Marowsky-Bree <lmb@suse.de> wrote:

The attached patch fixes drbds useage of bios up some. The proper fix
would be to indeed change it over to use bio_alloc(), bio_get/put(),
bio_add_page(), bio_clone (instead of __bio_clone) et cetera, but that
fix is too complex for the timeframe I have right now.

This should keep drbd-0.7.8 from oopsing not only on the SLES9 SP1
kernel but also the recent 2.6.10-ac series.

(I could possibly _code_ it, but it'd be too invasive and I'm weary of
the side-effects it might have and the QA would take too long. There's a
number of potential cleanups like further consolidation between
drbd_prepare_req_write/_read and others, but I'd propose to do that for
the drbd-0.8 branch instead when we can do away with 2.4.)

Please comment on the patch, I'd be grateful.

I have tested the patch in a standalone configuration, primary/secondary
(and switching around during resyncing etc), and it seems to be doing
quite well so far.


Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business


[-- Attachment #1.2: drbd-50103 --]
[-- Type: text/plain, Size: 6890 bytes --]

Index: drbd/drbd_actlog.c
===================================================================
--- drbd/drbd_actlog.c	(revision 1730)
+++ drbd/drbd_actlog.c	(working copy)
@@ -64,35 +64,29 @@
 STATIC int _drbd_md_sync_page_io(drbd_dev *mdev, struct page *page, 
 				 sector_t sector, int rw, int size)
 {
-	struct bio bio;
-	struct bio_vec vec;
+	struct bio *bio = bio_alloc(GFP_KERNEL, 1);
 	struct completion event;
 	int ok;
 
-	bio_init(&bio);
-	bio.bi_io_vec = &vec;
-	vec.bv_page = page;
-	vec.bv_offset = 0;
-	vec.bv_len =
-	bio.bi_size = size;
-	bio.bi_vcnt = 1;
-	bio.bi_idx = 0;
-	bio.bi_bdev = mdev->md_bdev;
-	bio.bi_sector = sector;
+	bio_get(bio);
+
+	bio->bi_bdev = mdev->md_bdev;
+	bio->bi_sector = sector;
+	bio_add_page(bio, page, size, 0);
 	init_completion(&event);
-	bio.bi_private = &event;
-	bio.bi_end_io = drbd_md_io_complete;
+	bio->bi_private = &event;
+	bio->bi_end_io = drbd_md_io_complete;
 
 #ifdef BIO_RW_SYNC
-	submit_bio(rw | (1 << BIO_RW_SYNC), &bio);
+	submit_bio(rw | (1 << BIO_RW_SYNC), bio);
 #else
-	submit_bio(rw, &bio);
+	submit_bio(rw, bio);
 	drbd_blk_run_queue(bdev_get_queue(mdev->md_bdev));
 #endif
 	wait_for_completion(&event);
 
-	ok = test_bit(BIO_UPTODATE, &bio.bi_flags);
-
+	ok = test_bit(BIO_UPTODATE, &bio->bi_flags);
+	bio_put(bio);
 	return ok;
 }
 #endif
Index: drbd/drbd_compat_wrappers.h
===================================================================
--- drbd/drbd_compat_wrappers.h	(revision 1730)
+++ drbd/drbd_compat_wrappers.h	(working copy)
@@ -429,11 +429,9 @@
  */
 static inline char *drbd_bio_kmap(struct bio *bio)
 {
-	struct bio_vec *bvec;
+	struct bio_vec *bvec = bio_iovec(bio);
 	unsigned long addr;
 
-	bvec = bio_iovec_idx(bio, bio->bi_idx);
-
 	addr = (unsigned long) kmap(bvec->bv_page);
 
 	if (addr & ~PAGE_MASK)
@@ -444,16 +442,15 @@
 
 static inline void drbd_bio_kunmap(struct bio *bio)
 {
-	struct bio_vec *bvec;
+	struct bio_vec *bvec = bio_iovec(bio);
 
-	bvec = bio_iovec_idx(bio, bio->bi_idx);
 	kunmap(bvec->bv_page);
 }
 
 #else
 static inline char *drbd_bio_kmap(struct bio *bio)
 {
-	struct bio_vec *bvec = bio_iovec_idx(bio, bio->bi_idx);
+	struct bio_vec *bvec = bio_iovec(bio);
 	return page_address(bvec->bv_page) + bvec->bv_offset;
 }
 static inline void drbd_bio_kunmap(struct bio *bio)
@@ -466,16 +463,17 @@
 {
 	struct bio * const bio = &e->private_bio;
 	struct bio_vec * const vec = &e->ee_bvec;
+
 	memset(e, 0, sizeof(*e));
+	bio_init(bio);
 
-	// bio_init(&bio); memset did it for us.
 	bio->bi_io_vec = vec;
-	vec->bv_page   = page;
-	vec->bv_len    =
-	bio->bi_size   = PAGE_SIZE;
-	bio->bi_max_vecs = 1;
 	bio->bi_destructor = NULL;
-	atomic_set(&bio->bi_cnt, 1);
+	vec->bv_page = page;
+	bio->bi_size = vec->bv_len = PAGE_SIZE;
+	bio->bi_max_vecs = bio->bi_vcnt = 
+	bio->bi_phys_segments = bio->bi_hw_segments = 1;
+	vec->bv_offset = 0;
 
 	e->block_id = ID_VACANT;
 }
@@ -495,20 +493,25 @@
 		    sector_t sector, int size)
 {
 	struct bio * const bio = &e->private_bio;
-
+	struct bio_vec * const vec = &e->ee_bvec;
+	struct page * const page = vec->bv_page;
 	D_ASSERT(mdev->backing_bdev);
 
-	bio->bi_flags  = 1 << BIO_UPTODATE;
-	bio->bi_io_vec->bv_len =
-	bio->bi_size    = size;
-	bio->bi_bdev    = mdev->backing_bdev;
-	bio->bi_sector  = sector;
+	/* Clear plate. */
+	bio_init(bio);
+
+	bio->bi_io_vec = vec;
+	bio->bi_destructor = NULL;
+	vec->bv_page = page;
+	vec->bv_offset = 0;
+	bio->bi_max_vecs = bio->bi_vcnt = 
+	bio->bi_phys_segments = bio->bi_hw_segments = 1;
+
+	bio->bi_bdev = mdev->backing_bdev;
 	bio->bi_private = mdev;
-	bio->bi_next    = 0;
-	bio->bi_idx     = 0; // for blk_recount_segments
-	bio->bi_vcnt    = 1; // for blk_recount_segments
-	e->ee_sector = sector;
-	e->ee_size = size;
+
+	e->ee_sector = bio->bi_sector = sector;
+	e->ee_size = bio->bi_size = bio->bi_io_vec->bv_len = size;
 }
 
 static inline void
@@ -530,15 +533,19 @@
 static inline void
 drbd_req_prepare_write(drbd_dev *mdev, struct drbd_request *req)
 {
-	struct bio * const bio     = &req->private_bio;
-	struct bio * const bio_src =  req->master_bio;
+	struct bio * const bio      = &req->private_bio;
+	struct bio_vec * const bvec = &req->req_bvec;
+	struct bio * const bio_src  =  req->master_bio;
 
 	bio_init(bio); // bio->bi_flags   = 0;
+	bio->bi_io_vec = bvec;
+	bio->bi_max_vecs = 1;
+
 	__bio_clone(bio,bio_src);
 	bio->bi_bdev    = mdev->backing_bdev;
 	bio->bi_private = mdev;
 	bio->bi_end_io  = drbd_dio_end;
-	bio->bi_next    = 0;
+	bio->bi_next    = NULL;
 
 	req->rq_status = RQ_DRBD_NOTHING;
 }
@@ -546,22 +553,26 @@
 static inline void
 drbd_req_prepare_read(drbd_dev *mdev, struct drbd_request *req)
 {
-	struct bio * const bio     = &req->private_bio;
-	struct bio * const bio_src =  req->master_bio;
+	struct bio * const bio      = &req->private_bio;
+	struct bio_vec * const bvec = &req->req_bvec;
+	struct bio * const bio_src  =  req->master_bio;
 
 	bio_init(bio); // bio->bi_flags   = 0;
+	bio->bi_io_vec = bvec;
+	bio->bi_max_vecs = 1;
+
 	__bio_clone(bio,bio_src);
 	bio->bi_bdev    = mdev->backing_bdev;
 	bio->bi_private = mdev;
 	bio->bi_end_io  = drbd_read_bi_end_io;	// <- only difference
-	bio->bi_next    = 0;
+	bio->bi_next    = NULL;
 
 	req->rq_status = RQ_DRBD_NOTHING;
 }
 
 static inline struct page* drbd_bio_get_page(struct bio *bio)
 {
-	struct bio_vec *bvec = bio_iovec_idx(bio, bio->bi_idx);
+	struct bio_vec *bvec = bio_iovec(bio);
 	return bvec->bv_page;
 }
 
@@ -622,13 +633,13 @@
 
 static inline int _drbd_send_zc_bio(drbd_dev *mdev, struct bio *bio)
 {
-	struct bio_vec *bvec = bio_iovec_idx(bio, bio->bi_idx);
+	struct bio_vec *bvec = bio_iovec(bio);
 	return _drbd_send_page(mdev,bvec->bv_page,bvec->bv_offset,bvec->bv_len);
 }
 
 static inline int _drbd_send_bio(drbd_dev *mdev, struct bio *bio)
 {
-	struct bio_vec *bvec = bio_iovec_idx(bio, bio->bi_idx);
+	struct bio_vec *bvec = bio_iovec(bio);
 	struct page *page = bvec->bv_page;
 	size_t size = bvec->bv_len;
 	int offset = bvec->bv_offset;
Index: drbd/drbd_int.h
===================================================================
--- drbd/drbd_int.h	(revision 1730)
+++ drbd/drbd_int.h	(working copy)
@@ -630,6 +630,7 @@
 	struct drbd_barrier *barrier; // The next barrier.
 	drbd_bio_t *master_bio;       // master bio pointer
 	drbd_bio_t private_bio;       // private bio struct
+	ONLY_IN_26(struct bio_vec req_bvec;)
 };
 
 struct drbd_barrier {
@@ -669,7 +670,7 @@
 	long magic;
 	ONLY_IN_26(unsigned int ee_size;)
 	ONLY_IN_26(sector_t ee_sector;)
-	// THINK: maybe we rather want bio_alloc(GFP_*,1)
+	// TODO: we rather want bio_alloc(GFP_*,1) all through the code!
 	ONLY_IN_26(struct bio_vec ee_bvec;)
 };
 

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]