lkml.org 
[lkml]   [2008]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/6] block: cfq: make the io contect sharing lockless
    On Wed, Jan 23 2008, Andrew Morton wrote:
    > > On Tue, 22 Jan 2008 10:49:19 +0100 Jens Axboe <jens.axboe@oracle.com> wrote:
    > > The io context sharing introduced a per-ioc spinlock, that would protect
    > > the cfq io context lookup. That is a regression from the original, since
    > > we never needed any locking there because the ioc/cic were process private.
    > >
    > > The cic lookup is changed from an rbtree construct to a radix tree, which
    > > we can then use RCU to make the reader side lockless. That is the performance
    > > critical path, modifying the radix tree is only done on process creation
    > > (when that process first does IO, actually) and on process exit (if that
    > > process has done IO).
    >
    > Perhaps Paul would review the rcu usage here sometime?

    That would indeed be awesome :-)

    > > +/*
    > > + * Add cic into ioc, using cfqd as the search key. This enables us to lookup
    > > + * the process specific cfq io context when entered from the block layer.
    > > + * Also adds the cic to a per-cfqd list, used when this queue is removed.
    > > + */
    > > +static inline int
    >
    > There's a lot of pointless inlining in there.

    Will kill.

    > > +++ b/block/ll_rw_blk.c
    > > @@ -3831,6 +3831,16 @@ int __init blk_dev_init(void)
    > > return 0;
    > > }
    > >
    > > +static void cfq_dtor(struct io_context *ioc)
    > > +{
    > > + struct cfq_io_context *cic[1];
    > > + int r;
    > > +
    > > + r = radix_tree_gang_lookup(&ioc->radix_root, (void **) cic, 0, 1);
    > > + if (r > 0)
    > > + cic[0]->dtor(ioc);
    > > +}
    >
    > Some comments here might help others who are wondering why we can't just
    > use radix_tree_lookup().

    Sure, will add a comment.

    > > @@ -3900,7 +3911,7 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
    > > ret->last_waited = jiffies; /* doesn't matter... */
    > > ret->nr_batch_requests = 0; /* because this is 0 */
    > > ret->aic = NULL;
    > > - ret->cic_root.rb_node = NULL;
    > > + INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC | __GFP_HIGH);
    >
    > Did this need to be atomic?

    It's actually only ever used with a radix_tree_preload() where the
    proper gfp mask is passed, the actual radix_tree_insert() is done under
    lock protecting the tree.

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2008-01-24 08:39    [W:0.020 / U:119.572 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site