Re: [PATCH 1/3] perf: Fix timechart header handling

[OGAWA Hirofumi]

Ingo Molnar <mingo@xxxxxxx> writes:

> * OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx> wrote:
>
>> Hi,
>> 
>> Update "struct trace_entry" to match with current one. And remove
>> "size" field from it.
>> 
>> If it has "size", it become cause of alignment mismatch of structure
>> with kernel.
>> 
>> Signed-off-by: OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
>> ---
>> 
>>  tools/perf/builtin-timechart.c |   12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Just curious, have you tested timechart functionality after having done 
> this change - does it still work fine for you?

Yes. Rather, it doesn't work without this. (Without patch, it provide
svg output, but it doesn't provide correct sched state. And this issue
is only 64bit arch.)

E.g. The following has difference (perf timechart doesn't has hole).

In kernel,

struct ftrace_raw_sched_switch {
	struct trace_entry         ent;                  /*     0    12 */
	char                       prev_comm[16];        /*    12    16 */
	pid_t                      prev_pid;             /*    28     4 */
	int                        prev_prio;            /*    32     4 */

	/* XXX 4 bytes hole, try to pack */

	long int                   prev_state;           /*    40     8 */
	char                       next_comm[16];        /*    48    16 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	pid_t                      next_pid;             /*    64     4 */
	int                        next_prio;            /*    68     4 */
	char                       __data[0];            /*    72     0 */

	/* size: 72, cachelines: 2, members: 9 */
	/* sum members: 68, holes: 1, sum holes: 4 */
	/* last cacheline: 8 bytes */
};

In perl timechart, [Note, struct trace_entry is including "u32 size"]

struct sched_switch {
	struct trace_entry         te;                   /*     0    16 */
	char                       prev_comm[16];        /*    16    16 */
	int                        prev_pid;             /*    32     4 */
	int                        prev_prio;            /*    36     4 */
	long int                   prev_state;           /*    40     8 */
	char                       next_comm[16];        /*    48    16 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	int                        next_pid;             /*    64     4 */
	int                        next_prio;            /*    68     4 */

	/* size: 72, cachelines: 2, members: 8 */
	/* last cacheline: 8 bytes */
};


Thanks.
-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/