lkml.org 
[lkml]   [2004]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 2.6: drivers/md/dm-io.c partially copies bio.c
    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

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:08    [W:0.026 / U:0.088 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site