linux-kernel - Re: [PATCH linux-next 2/3] sched/psi: Avoid update triggers and rtpoll

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:   Mon, 9 Oct 2023 10:42:08 -0700
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     yang.yang29@....com.cn, peterz@...radead.org, hannes@...xchg.org,
        mingo@...hat.com, linux-kernel@...r.kernel.org,
        juri.lelli@...hat.com
Subject: Re: [PATCH linux-next 2/3] sched/psi: Avoid update triggers and
 rtpoll_total when it is unnecessary

On Mon, Oct 9, 2023 at 5:52 AM Ingo Molnar <mingo@...nel.org> wrote:
>
>
> * yang.yang29@....com.cn <yang.yang29@....com.cn> wrote:
>
> > From: Yang Yang <yang.yang29@....com.cn>
> >
> > When psimon wakes up and there are no state changes for rtpoll_states,
> > it's unnecessary to update triggers and rtpoll_total because the pressures
> > being monitored by user had not changed. This will help to slightly reduce
> > unnecessary computations of psi.
> >
> > And update group->rtpoll_next_update after called update_triggers() and
> > update rtpoll_total. This will prevent bugs if update_triggers() uses
> > group->rtpoll_next_update in the future, and it makes more sense
> > to set the next update time after we finished the current update.
>
> >       if (now >= group->rtpoll_next_update) {
> > -             update_triggers(group, now, &update_total, PSI_POLL);
> > -             group->rtpoll_next_update = now + group->rtpoll_min_period;
> > -             if (update_total)
> > +             if (changed_states & group->rtpoll_states) {
> > +                     update_triggers(group, now, &update_total, PSI_POLL);
> >                       memcpy(group->rtpoll_total, group->total[PSI_POLL],
> >                                  sizeof(group->rtpoll_total));
> > +             }
> > +             group->rtpoll_next_update = now + group->rtpoll_min_period;
>
> So please also split out the second change into a separate patch as well,
> as it's an unrelated patch to the state-change optimization.

I think that the second part could have been done in the first patch
to place the "group->rtpoll_next_update = now +
group->rtpoll_min_period" line at the right place from the beginning.
Also when posting the next version please add the version number to
all the patch titles in the patchset, not only to the cover letter.
That helps with finding the latest version.
Thanks!

>
> We have a "one conceptual change per patch" rule for most things.
>
> Thanks,
>
>         Ingo