linux-kernel - Re: [PATCH v2 01/26] block: Convert integrity to bvec_alloc

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: <20120920215313.GE7264@google.com>
Date:	Thu, 20 Sep 2012 14:53:13 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Kent Overstreet <koverstreet@...gle.com>
Cc:	linux-bcache@...r.kernel.org, linux-kernel@...r.kernel.org,
	dm-devel@...hat.com, axboe@...nel.dk, neilb@...e.de,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(),
 and a bugfix

Hello,

On Mon, Sep 10, 2012 at 05:22:12PM -0700, Kent Overstreet wrote:
> This adds a pointer to the bvec array to struct bio_integrity_payload,
> instead of the bvecs always being inline; then the bvecs are allocated
> with bvec_alloc_bs().
> 
> This is needed eventually for immutable bio vecs - immutable bvecs
> aren't useful if we still have to copy them, hence the need for the
> pointer. Less code is always nice too, though.
> 
> Also fix an amusing bug in bio_integrity_split() - struct bio_pair
> doesn't have the integrity bvecs after the bio_integrity_payloads, so
> there was a buffer overrun. The code was confusing pointers with arrays.

Aside from what Martin and Vivek already pointed out, it generally
looks okay to me but here are some thoughts.

I'm quite doubtful how much we're gaining by this complex set of slabs
and mempools approach compared to just using kmalloc + one mempool for
the largest size.  I can't think of anything to be gained in terms of
cacheline hotness.  In terms of memory usage too, we're proabably
introducing more fragmentation with all the different slabs.

Jens, what are you thoughts?  For forward progress guarantee, single
mempool per bioset serving the largest one is enough and we can
significantly simplify the whole bioset thing.

Thanks.

-- 
tejun
--
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/