lkml.org 
[lkml]   [2011]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 05/10] block: remove per-queue plugging
    From
    Date
    On Sat, 2011-03-05 at 21:54 +0100, Jens Axboe wrote:
    >
    > Apparently so. Peter/Ingo, please shoot this one down in flames.
    > Summary:
    >
    > - Need a way to trigger this flushing when a task is going to sleep
    > - It's currently done right before calling deactivate_task(). We know
    > the task is going to sleep here, but it's also under the runqueue
    > lock. Not good.
    > - In the new location, it's not completely clear to me whether we can
    > safely deref 'prev' or not. The usage of prev_state would seem to
    > indicate that we cannot, and as far as I can tell, prev could at this
    > point already potentially be running on another CPU.
    >
    > Help? Peter, we talked about this in Tokyo in September. Initial
    > suggestion was to use preempt notifiers, which we can't because:
    >
    > - runqueue lock is also held
    > - It's not unconditionally available, depends on config.
    >
    > diff --git a/kernel/sched.c b/kernel/sched.c
    > index e806446..8581ad3 100644
    > --- a/kernel/sched.c
    > +++ b/kernel/sched.c
    > @@ -2826,6 +2826,14 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
    > #endif /* __ARCH_WANT_INTERRUPTS_ON_CTXSW */
    > finish_lock_switch(rq, prev);
    >
    > + /*
    > + * If this task has IO plugged, make sure it
    > + * gets flushed out to the devices before we go
    > + * to sleep
    > + */
    > + if (prev_state != TASK_RUNNING)
    > + blk_flush_plug(prev);
    > +
    > fire_sched_in_preempt_notifiers(current);
    > if (mm)
    > mmdrop(mm);
    > @@ -3973,14 +3981,6 @@ need_resched_nonpreemptible:
    > if (to_wakeup)
    > try_to_wake_up_local(to_wakeup);
    > }
    > - /*
    > - * If this task has IO plugged, make sure it
    > - * gets flushed out to the devices before we go
    > - * to sleep
    > - */
    > - blk_flush_plug(prev);
    > - BUG_ON(prev->plug && !list_empty(&prev->plug->list));
    > -
    > deactivate_task(rq, prev, DEQUEUE_SLEEP);
    > }
    > switch_count = &prev->nvcsw;
    >

    Right, so your new location is still under rq->lock for a number of
    architectures (including x86). finish_lock_switch() doesn't actually
    release the lock unless __ARCH_WANT_INTERRUPTS_ON_CTXSW ||
    __ARCH_WANT_UNLOCKED_CTXSW (the former implies the latter since rq->lock
    is IRQ-safe).

    If you want a safe place to drop rq->lock (but keep in mind to keep IRQs
    disabled there) and use prev, do something like the below. Both
    pre_schedule() and idle_balance() can already drop the rq->lock do doing
    it once more is quite all-right ;-)

    Note that once you drop rq->lock prev->state can change to TASK_RUNNING
    again so don't re-check that.

    ---
    kernel/sched.c | 6 +++++-
    1 files changed, 5 insertions(+), 1 deletions(-)

    diff --git a/kernel/sched.c b/kernel/sched.c
    index 655164e..99c5637 100644
    --- a/kernel/sched.c
    +++ b/kernel/sched.c
    @@ -4120,8 +4120,12 @@ need_resched_nonpreemptible:
    switch_count = &prev->nvcsw;
    }

    + if (prev->state != TASK_RUNNING) {
    + raw_spin_unlock(&rq->lock);
    + blk_flush_plug(prev);
    + raw_spin_lock(&rq->lock);
    + }
    pre_schedule(rq, prev);
    -
    if (unlikely(!rq->nr_running))
    idle_balance(cpu, rq);



    \
     
     \ /
      Last update: 2011-03-07 11:27    [W:4.025 / U:0.360 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site