On Mon, Dec 06 2004, Kevin Corry wrote: > 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. That is fully correct and also something that eg the bouncing code needs to be 100% deadlock free. > 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. The mempools are needed, the duplicated slabs are not. But that's just a small part of it, basically the whole allocation and index stuff is 100% duplicated from bio.c. Even bio_init(). > 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 An abstraction for this would be nice, as there are two users of it then (dm-io and highmem.c). So if your team would do that, I would sure help you review and integrate it. That's not what I'm arguing against, it's the (almost) wholesale duplication that's a bit silly. -- Jens Axboe