lkml.org 
[lkml]   [2011]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 5/5] CFQ: use proper locking for cache of last hit cic
    From
    Date
    On Mon, 2011-06-06 at 05:06 +0200, Jens Axboe wrote:
    > On 2011-06-05 18:26, Paul Bolle wrote:
    > > @@ -2704,8 +2706,13 @@ static void __cfq_exit_single_io_context(struct cfq_data *cfqd,
    > > smp_wmb();
    > > cic->key = cfqd_dead_key(cfqd);
    > >
    > > - if (ioc->last_cic == cic)
    > > + spin_lock_irqsave(&ioc->lock, flags);
    > > + rcu_read_lock();
    > > + last_cic = rcu_dereference(ioc->last_cic);
    > > + rcu_read_unlock();
    > > + if (last_cic == cic)
    > > rcu_assign_pointer(ioc->last_cic, NULL);
    > > + spin_unlock_irqrestore(&ioc->lock, flags);
    >
    > We don't need the ioc->lock for checking the cache, it would in fact
    > defeat the purpose of using RCU.

    Just to show that I'm RCU-challenged, is that because:
    1) my use of locking on ioc->lock defends for a race that is not
    actually possible; or
    2) the worst thing that could happen is that some new and correct value
    of ioc->last_cic will be replaced with NULL, which is simply not a big
    deal?

    > But this hunk will clash with the
    > merged part anyway.

    Looking at Paul's feedback I do have a feeling that in your commit
    (9b50902db5eb8a220160fb89e95aa11967998d12, "cfq-iosched: fix locking
    around ioc->ioc_data assignment") the line:
    if (rcu_dereference(ioc->ioc_data) == cic) {

    could actually be replaced with:
    if (rcu_access_pointer(ioc->ioc_data) == cic) {

    Is that correct?

    > [...]
    > See Pauls comment on this part.

    You seem to be offline right now. When you're back online, could you
    please say whether or not you accept the two renaming patches that you
    have rejected a few days ago (and for which I gave some follow up
    arguments)? After that I'll send an update to this patch (and its commit
    message) to reflect Paul's review and your review.


    Paul Bolle



    \
     
     \ /
      Last update: 2011-06-08 20:21    [W:0.032 / U:30.140 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site