Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1585904125-2819-1-git-send-email-laoar.shao@gmail.com> <20200403152016.GA69203@cmpxchg.org>
In-Reply-To: <20200403152016.GA69203@cmpxchg.org>
From:   Yafang Shao <laoar.shao@gmail.com>
Date:   Sat, 4 Apr 2020 10:31:16 +0800
Message-ID: <CALOAHbBqk4s4Nw3Ky3YY66EM7DzUJZTuLZhjw9+ULUE-9O+aDQ@mail.gmail.com>
Subject: Re: [PATCH] psi: fix randomized calculation in record_times()
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux MM <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Daniel Drake <drake@endlessm.com>,
        Suren Baghdasaryan <surenb@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Apr 3, 2020 at 11:20 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Fri, Apr 03, 2020 at 04:55:25AM -0400, Yafang Shao wrote:
> > @@ -690,7 +690,10 @@ static void psi_group_change(struct psi_group *group, int cpu,
> >        */
> >       write_seqcount_begin(&groupc->seq);
> >
> > -     record_times(groupc, cpu, false);
> > +     if (groupc->total_tasks)
> > +             record_times(groupc, cpu, false);
> > +     else
> > +             groupc->state_start = cpu_clock(cpu);
>
> This change appears is a no-op. If there are no tasks,
> groupc->state_mask is 0, and the only thing record_times() does is
>
>         groupc->state_start = cpu_clock(cpu);
>
> Did you encounter actual problems, or are you just reading the code?

No real issues. Just reading the code and found it looks like a little weird.
But as you explained that state_mask can also guarantee it then we
don't need to make this change.

Thanks for the explanation.

Thanks
Yafang