linux-kernel - Re: [PATCH next] bio: zero inlined bio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0812231110320.9177@blonde.anvils>
Date:	Tue, 23 Dec 2008 11:21:22 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	Jens Axboe <jens.axboe@...cle.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH next] bio: zero inlined bio_vec

On Tue, 23 Dec 2008, Jens Axboe wrote:
> On Tue, Dec 23 2008, Jens Axboe wrote:
> > 
> > That is, it iterates *bio_orig but indexes bio since it knows it has the
> > same number of segments. So this code is the odd one out, I'd be
> > surprised if we had more such cases. And since it would be nice to get
> > rid of the need to memset in general, can you try with the below patch?
> 
> Nope, that still wont be enough, since we leave entries 0..i-1
> uninitialized. Lets just do this instead.

Yes, that second version worked for me.  But if __blk_queue_bounce()
is indeed the odd one out (likely but not certain), then the extension
below makes more sense - I don't like how your bio.c returns a cleared
bio_vec in some cases but not in others.  This is running fine for me
on my test machines, but my test coverage is probably very poor (am I
ever using more than the inlined bio_vec?), and I've not really tried
to understand all the ways through bvec_alloc_bs().

Hugh

--- a/fs/bio.c
+++ b/fs/bio.c
@@ -180,7 +180,7 @@ struct bio_vec *bvec_alloc_bs(gfp_t gfp_
 	 * kzalloc() for the exact number of vecs right away.
 	 */
 	if (!bs)
-		bvl = kzalloc(nr * sizeof(struct bio_vec), gfp_mask);
+		bvl = kmalloc(nr * sizeof(struct bio_vec), gfp_mask);
 
 	/*
 	 * see comment near bvec_array define!
@@ -237,9 +237,6 @@ fallback:
 		}
 	}
 
-	if (bvl)
-		memset(bvl, 0, bvec_nr_vecs(*idx) * sizeof(struct bio_vec));
-
 	return bvl;
 }
 
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -198,11 +198,14 @@ static void __blk_queue_bounce(struct re
 		/*
 		 * irk, bounce it
 		 */
-		if (!bio)
-			bio = bio_alloc(GFP_NOIO, (*bio_orig)->bi_vcnt);
+		if (!bio) {
+			unsigned int vcnt = (*bio_orig)->bi_vcnt;
 
-		to = bio->bi_io_vec + i;
+			bio = bio_alloc(GFP_NOIO, vcnt);
+			memset(bio->bi_io_vec, 0,vcnt * sizeof(struct bio_vec));
+		}
 
+		to = bio->bi_io_vec + i;
 		to->bv_page = mempool_alloc(pool, q->bounce_gfp);
 		to->bv_len = from->bv_len;
 		to->bv_offset = from->bv_offset;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/