Re: [PATCH linux-next 2/3] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary

[Suren Baghdasaryan]

On Mon, Oct 9, 2023 at 5:52 AM Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
>
> * yang.yang29@xxxxxxxxxx <yang.yang29@xxxxxxxxxx> wrote:
>
> > From: Yang Yang <yang.yang29@xxxxxxxxxx>
> >
> > 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