All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@kernel.org, markus@trippelsdorf.de, tj@kernel.org,
	mcgrof@kernel.org, ebiederm@xmission.com,
	paulmck@linux.vnet.ibm.com, torvalds@linux-foundation.org,
	tglx@linutronix.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/8] sched: Consistent task-state printing
Date: Mon, 25 Sep 2017 16:01:09 -0400	[thread overview]
Message-ID: <20170925160109.0086eccb@vmware.local.home> (raw)
In-Reply-To: <20170925121116.927982688@infradead.org>

On Mon, 25 Sep 2017 14:07:48 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> +static inline char __task_state_to_char(unsigned int state)
> +{
> +	static const char state_char[] = "RSDTtXZ";
> +
> +	BUILD_BUG_ON(1 + ilog2(TASK_REPORT) != sizeof(state_char) - 2);
>  
> -	return state < sizeof(stat_nam) - 1 ? stat_nam[state] : '?';
> +	return state_char[state];
> +}
> +
> +static inline char task_state_to_char(struct task_struct *tsk)
> +{
> +	return __task_state_to_char(__get_task_state(tsk));
>  }
>  

So far I'm fine with the patch set, but I hate the non descriptive "__"
prefix of __task_state_to_char(). Can we make this a bit more
descriptive, because every time I see it in other patches, I go back to
this patch to see if we are using the right function.

What about something like:

task_state_to_state_char(unsigned int state);

task_to_state_char(struct task_struct *tsk);

?

This way, the "__" wont keep making me think we used the wrong
function.

-- Steve

  reply	other threads:[~2017-09-25 20:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-25 12:07 [PATCH 0/8] sched: Rework task state printing Peter Zijlstra
2017-09-25 12:07 ` [PATCH 1/8] sched: Consistent task-state printing Peter Zijlstra
2017-09-25 20:01   ` Steven Rostedt [this message]
2017-09-29 11:50     ` Peter Zijlstra
2017-09-29 13:00       ` Steven Rostedt
2017-10-10 10:55       ` [tip:sched/core] sched/debug: Rename task-state printing helpers tip-bot for Peter Zijlstra
2017-09-25 12:07 ` [PATCH 2/8] sched: Convert TASK_state to hex Peter Zijlstra
2017-09-25 12:07 ` [PATCH 3/8] sched/debug: Remove unused variable Peter Zijlstra
2017-09-25 12:07 ` [PATCH 4/8] sched/trace: Fix trace_sched_switch task-state printing Peter Zijlstra
2017-09-25 12:07 ` [PATCH 5/8] sched/trace: Use common task-state helpers Peter Zijlstra
2017-09-25 12:07 ` [PATCH 6/8] sched: Add explicit TASK_IDLE printing Peter Zijlstra
2017-09-25 12:07 ` [PATCH 7/8] sched: Ignore TASK_IDLE for SysRq-W Peter Zijlstra
2017-09-25 12:07 ` [PATCH 8/8] sched: Add explicit TASK_PARKED printing Peter Zijlstra
2017-09-25 13:41 ` [PATCH 0/8] sched: Rework task state printing Thomas Gleixner

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=20170925160109.0086eccb@vmware.local.home \
    --to=rostedt@goodmis.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus@trippelsdorf.de \
    --cc=mcgrof@kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.