linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Suren Baghdasaryan <surenb@google.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: yang.yang29@zte.com.cn, peterz@infradead.org, hannes@cmpxchg.org,
	mingo@redhat.com, linux-kernel@vger.kernel.org,
	juri.lelli@redhat.com
Subject: Re: [PATCH linux-next 2/3] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary
Date: Mon, 9 Oct 2023 10:42:08 -0700	[thread overview]
Message-ID: <CAJuCfpFabCn8gcuLV322RKC=xzVm0C+64HQP+CkFNJZ4VO42ZA@mail.gmail.com> (raw)
In-Reply-To: <ZSP3cuEsgwWcIKRw@gmail.com>

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

  reply	other threads:[~2023-10-09 17:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-13 12:44 [PATCH linux-next] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary yang.yang29
2023-09-13 23:01 ` Suren Baghdasaryan
2023-09-14  2:02   ` yang.yang29
2023-09-14  5:53     ` yang.yang29
2023-09-14  9:10       ` yang.yang29
2023-09-27  0:14         ` Suren Baghdasaryan
2023-09-27  1:56           ` yang.yang29
2023-09-30 22:32         ` Suren Baghdasaryan
2023-10-07  6:21           ` [PATCH linux-next v2] " yang.yang29
2023-10-09 10:49             ` Ingo Molnar
2023-10-09 12:19               ` [PATCH linux-next 0/3] sched/psi: Optimize the process of updating triggers and rtpoll_total yang.yang29
2023-10-09 12:24               ` [PATCH linux-next 1/3] sched/psi: Change update_triggers() to a void function yang.yang29
2023-10-09 12:48                 ` Ingo Molnar
2023-10-09 12:59                 ` [tip: sched/core] sched/psi: Change update_triggers() to a 'void' function tip-bot2 for Yang Yang
2023-10-09 12:30               ` [PATCH linux-next 2/3] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary yang.yang29
2023-10-09 12:52                 ` Ingo Molnar
2023-10-09 17:42                   ` Suren Baghdasaryan [this message]
2023-10-10  2:12                     ` yang.yang29
2023-10-10  3:05                     ` [PATCH linux-next v2 0/4] sched/psi: Optimize the process of updating triggers and rtpoll_total yang.yang29
2023-10-10  3:09                     ` [PATCH linux-next v2 1/4] sched/psi: Change update_triggers() to a 'void' function yang.yang29
2023-10-10  7:38                       ` Ingo Molnar
2023-10-10  8:31                         ` [PATCH linux-next v3 0/4] sched/psi: Optimize the process of updating triggers and rtpoll_total yang.yang29
2023-10-10  8:36                         ` [PATCH linux-next v3 1/4] sched/psi: Change update_triggers() to a 'void' function yang.yang29
2023-10-19 16:01                           ` Suren Baghdasaryan
2023-10-10  8:41                         ` [PATCH linux-next v3 2/4] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary yang.yang29
2023-10-11 21:15                           ` Ingo Molnar
2023-10-12  2:13                             ` yang.yang29
2023-10-19 16:04                             ` Suren Baghdasaryan
2023-10-11 21:20                           ` [tip: sched/core] sched/psi: Avoid updating PSI triggers and ->rtpoll_total when there are no state changes tip-bot2 for Yang Yang
2023-10-10  8:42                         ` [PATCH linux-next v3 3/4] sched/psi: update rtpoll_next_update after update triggers and rtpoll_total yang.yang29
2023-10-19 16:07                           ` Suren Baghdasaryan
2023-10-10  8:45                         ` [PATCH linux-next v3 4/4] sched/psi: Delete the function parameter update_total of update_triggers() yang.yang29
2023-10-11 21:20                           ` [tip: sched/core] sched/psi: Delete the 'update_total' function parameter from update_triggers() tip-bot2 for Yang Yang
2023-10-19 16:09                           ` [PATCH linux-next v3 4/4] sched/psi: Delete the function parameter update_total of update_triggers() Suren Baghdasaryan
2023-10-10  3:20                     ` [PATCH linux-next v2 2/4] sched/psi: Avoid update triggers and rtpoll_total when it is unnecessary yang.yang29
2023-10-10  3:24                     ` [PATCH linux-next v2 3/4] sched/psi: update rtpoll_next_update after update triggers and rtpoll_total yang.yang29
2023-10-10  3:28                     ` [PATCH linux-next v2 4/4] sched/psi: Delete the function parameter update_total of update_triggers() yang.yang29
2023-10-09 12:37               ` [PATCH linux-next 3/3] " yang.yang29

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJuCfpFabCn8gcuLV322RKC=xzVm0C+64HQP+CkFNJZ4VO42ZA@mail.gmail.com' \
    --to=surenb@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=yang.yang29@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).