[dm-devel] Re: 2.6: drivers/md/dm-io.c partially copies bio.c

[Kevin Corry]

On Monday 06 December 2004 7:55 am, Jens Axboe wrote:
> On Mon, Dec 06 2004, Kevin Corry wrote:
> > Hi Adrian,
> >
> > On Monday 06 December 2004 6:09 am, Adrian Bunk wrote:
> > > drivers/md/dm-io.c copies functionality from bio.c .
> > >
> > > Is there a specific reason why you don't simply use the functionality
> > > bio.c provides?
> >
> > Can you give some specific examples of the functionality you think is
> > duplicated? Meanwhile, I'll take a look and see if I can explain any code
> > overlaps.
>
> Ah come on Kevin, a 2 second glance shows lots of uneccesary
> duplication. Basically only the concept of the bio_set is not duplicated
> in the first many lines, you even set up matching slabs.
>
> How was that ever accepted for merging?

If I recall correctly (and it's been a while since I've looked at that code), 
the bio_set was added because a few DM modules like to initiate their own I/O 
requests (things like snapshots and DM's kcopyd daemon), and we felt it was 
better to allow these modules to each create their own mempools to allocate 
bios from, rather than allocate from the single kernel-wide bio pool used by 
the filesystem layer.

Strictly speaking (and as you mentioned), the slabs in the bio_set are 
unnecessary, and they could use the global bio_slab. But there's probably 
some argument to be made for having separate bio mempools for these modules.

Actually, I also seem to recall discussions with Joe Thornber from quite 
awhile ago about trying to move this bio_set functionality into fs/bio.c, to 
allow other device drivers to create their own private bio pools. If you 
think something like this would be desireable, I can try to look into the 
specifics again. If you think that having the single kernel-wide bio pool is 
sufficient for all filesystems and device-drivers (you certainly understand 
the trade-offs better than I do), then I can look into removing the necessary 
code from dm-io.c

-- 
Kevin Corry
kevcorry at us.ibm.com
http://evms.sourceforge.net/