Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230803083352.1585-1-zegao@tencent.com> <20230803083352.1585-2-zegao@tencent.com>
 <20230803050900.5007d2f5@gandalf.local.home> <CAD8CoPDNEyf9LN8HfOTUR7c5npPPivJe8yncTWr756ZzPyZkNQ@mail.gmail.com>
In-Reply-To: <CAD8CoPDNEyf9LN8HfOTUR7c5npPPivJe8yncTWr756ZzPyZkNQ@mail.gmail.com>
From:   Ze Gao <zegao2021@gmail.com>
Date:   Thu, 3 Aug 2023 20:25:53 +0800
Message-ID: <CAD8CoPBuXY7s6GXH1na-AB595U4u_t0JJB3uUchGr-0KijVS4Q@mail.gmail.com>
Subject: Re: [RFC PATCH v6 1/5] perf sched: sync state char array with the kernel
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Adrian Hunter <adrian.hunter@intel.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Ian Rogers <irogers@google.com>,
        Ingo Molnar <mingo@redhat.com>, Jiri Olsa <jolsa@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, Ze Gao <zegao@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

THIS IS THE NEW CHANGELOG FOR THIS PATCH:

    perf sched: sync state char array with the kernel

    Since commit e936e8e459e14 ("perf tools: Adapt the
    TASK_STATE_TO_CHAR_STR to new value in kernel space."),
    the state char array that is used to interpret the
    states of tasks being switched out have not synced
    once with kernel definitions. Whereas the task report
    logic is evolving over this time and the definition
    of this state char array has been changed multiple
    times. And this leads to inconsistency.

    As of this writing, perf timehist --state still reports
    the wrong states because TASK_STATE_TO_CHAR_STR is too
    outdated to use.

    So sync TASK_STATE_TO_CHAR_STR to match the latest kernel
    definitions to fix it.

    Signed-off-by: Ze Gao <zegao@tencent.com>

Regards,
Ze

On Thu, Aug 3, 2023 at 6:29=E2=80=AFPM Ze Gao <zegao2021@gmail.com> wrote:
>
> On Thu, Aug 3, 2023 at 5:09=E2=80=AFPM Steven Rostedt <rostedt@goodmis.or=
g> wrote:
> >
> > On Thu,  3 Aug 2023 04:33:48 -0400
> > Ze Gao <zegao2021@gmail.com> wrote:
> >
> > Hi Ze,
> >
> > > Update state char array and then remove unused and stale
> > > macros, which are kernel internal representations and not
> > > encouraged to use anymore.
> > >
> >
> > A couple of things.
> >
> > First, the change logs of every commit need to specify the "why". The
> > subject can say "what", but the change log really needs to explain why =
this
> > patch is important. For example, this patch is really two changes (and =
thus
> > should actually be two patches). (I'll also comment on the other patche=
s)
>
> Thanks for the feedback! Will elaborate the changes in each changelog.
>
> > 1. The update of the state char array. You should explain why it's bein=
g
> > updated. If it was wrong, it needs to state the commit that changed to =
make
> > that happen.
> >
> > 2. For the removing the stale macros, the change log can simply state t=
hat
> > the macros are unused in the code and are being removed.
> >
> > Finally, I know you're eager to get this patch set in, but please hold =
off
> > sending a new version immediately after a comment or two. Some maintain=
ers
> > prefer submitters to wait a week or so, otherwise you will tend to "spa=
m"
> > their inboxes. There's more than one maintainer Cc'd on this series, an=
d you
> > need to be courteous not to send too many emails in a short period of t=
ime.
>
> Noted!  Actually I'm in no rush and just to make sure people see the
> latest patches so they do not have to waste time on the old series.
>
> Will hold off to resolve all the comments in this thread.
>
> And thanks for pointing this out.
>
> Regards,
> Ze