From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 24 Jan 2005 10:28:22 +0100 From: Lars Marowsky-Bree To: Jens Axboe Message-ID: <20050124092822.GB5451@marowsky-bree.de> References: <20050123161633.GH24350@marowsky-bree.de> <20050124091006.GA2716@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20050124091006.GA2716@suse.de> Cc: drbd-dev@lists.linbit.com Subject: [Drbd-dev] Re: drbd uses wrong API for struct bio List-Id: Coordination of development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 2005-01-24T10:10:07, Jens Axboe wrote: > drbd_ee_init() doesn't look good. First of all, it makes assumptions > about what bio_init() would do - it doesn't clear the entire bio to 0. > This is the type of thing that exposes bugs when we add or change parts > of the bio stuff, please uncomment that bio_init() call. It does set > bio->bi_max_vecs, however it needs changing to passing the bdev in so we > can use bio_add_page() instead. drbd_ee_bio_prepare() looks strange, it > sets vec length but doesn't assign the page or offset. Again, it should > use the proper api (bio_add_page()). If it had used bio_init(), it would > not have to set BIO_UPTODATE manually either. > > bio->bi_next is a pointer, not an integer. Use NULL. > > drbd_blk_run_queue() doesn't look legal, unless it's only ever run on > drbd's own queue. Even if so, it should remove the plug. I'd suggest > just using generic_unplug_device(). > > drdb should just use bio_iovec(bio) when it uses bio->bi_idx as the > index anyways. Ok, I'll go through the code and clean this up now. Thanks! I'll hassle you with more questions as my changes oops ;-) > The bio embedding may not be the best idea or the prettiest, but as long > as it sets ->bi_max_vecs it should be ok. In general the code could > really do with someone cleaning it up in preperation for submitting to > mainline... Yeah, that's happening with 0.8, but this is a 'maintenance' branch, with all what that entails :-( drbd 0.7 on 2.4 is (unfortunately ;-) still making money for Linbit, or else I'd guess everyone would be just too happy to rip it out. Sincerely, Lars Marowsky-Brée -- High Availability & Clustering SUSE Labs, Research and Development SUSE LINUX Products GmbH - A Novell Business