Messages in this thread | | | Date | Thu, 3 Aug 2023 05:09:00 -0400 | From | Steven Rostedt <> | Subject | Re: [RFC PATCH v6 1/5] perf sched: sync state char array with the kernel |
| |
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 patches)
1. The update of the state char array. You should explain why it's being 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 that 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 maintainers prefer submitters to wait a week or so, otherwise you will tend to "spam" their inboxes. There's more than one maintainer Cc'd on this series, and you need to be courteous not to send too many emails in a short period of time.
-- Steve
> Signed-off-by: Ze Gao <zegao@tencent.com> > --- > tools/perf/builtin-sched.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index 9ab300b6f131..8dc8f071721c 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -92,23 +92,12 @@ struct sched_atom { > struct task_desc *wakee; > }; > > -#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKWP" > +#define TASK_STATE_TO_CHAR_STR "RSDTtXZPI" > > /* task state bitmask, copied from include/linux/sched.h */ > #define TASK_RUNNING 0 > #define TASK_INTERRUPTIBLE 1 > #define TASK_UNINTERRUPTIBLE 2 > -#define __TASK_STOPPED 4 > -#define __TASK_TRACED 8 > -/* in tsk->exit_state */ > -#define EXIT_DEAD 16 > -#define EXIT_ZOMBIE 32 > -#define EXIT_TRACE (EXIT_ZOMBIE | EXIT_DEAD) > -/* in tsk->state again */ > -#define TASK_DEAD 64 > -#define TASK_WAKEKILL 128 > -#define TASK_WAKING 256 > -#define TASK_PARKED 512 > > enum thread_state { > THREAD_SLEEPING = 0,
| |