linux-kernel - Re: [PATCH 1/4] sched: move IO scheduling accounting from io_schedule_timeout() to _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Nov 2016 17:51:02 -0500
From:   Tejun Heo <tj@...nel.org>
To:     Pavan Kondeti <pkondeti@...eaurora.org>
Cc:     torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
        mingo@...hat.com, Peter Zijlstra <peterz@...radead.org>,
        axboe@...nel.dk, tytso@....edu, jack@...e.com,
        adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...com, mingbo@...com
Subject: Re: [PATCH 1/4] sched: move IO scheduling accounting from
 io_schedule_timeout() to __schedule()

Hello,

On Thu, Nov 03, 2016 at 09:03:45PM +0530, Pavan Kondeti wrote:
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 94732d1..f6baa38 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3336,11 +3336,17 @@ static void __sched notrace __schedule(bool preempt)
> >         unsigned long *switch_count;
> >         struct pin_cookie cookie;
> >         struct rq *rq;
> > -       int cpu;
> > +       int cpu, in_iowait;
> >
> >         cpu = smp_processor_id();
> >         rq = cpu_rq(cpu);
> >         prev = rq->curr;
> > +       in_iowait = prev->in_iowait;
> > +
> > +       if (in_iowait) {
> > +               delayacct_blkio_start();
> > +               atomic_inc(&rq->nr_iowait);
> > +       }
> >
> >         schedule_debug(prev);
> >
> > @@ -3406,6 +3412,11 @@ static void __sched notrace __schedule(bool preempt)
> >         }
> >
> >         balance_callback(rq);
> > +
> > +       if (in_iowait) {
> > +               atomic_dec(&rq->nr_iowait);
> > +               delayacct_blkio_end();
> > +       }
> >  }
> 
> I think, the nr_iowait update can go wrong here.
> 
> When the task migrates to a different CPU upon wakeup, this rq points
> to a different CPU from the one on which nr_iowait is incremented
> before.

Ah, you're right, it should remember the original rq.

Thanks.

-- 
tejun