| Message ID | 20191218074427.96184-4-svens@linux.ibm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/3] selftests/ftrace: fix glob selftest | expand |
On Wed, 18 Dec 2019 08:44:27 +0100 Sven Schnelle <svens@linux.ibm.com> wrote: > At least on PA-RISC and s390 synthetic histogram triggers are failing > selftests because trace_event_raw_event_synth() always writes a 64 bit > values, but the reader expects a field->size sized value. On little endian > machines this doesn't hurt, but on big endian this makes the reader always > read zero values. Does this fix make the other one obsolete? https://lore.kernel.org/r/20191211110959.2baeb70f@gandalf.local.home -- Steve > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > --- > kernel/trace/trace_events_hist.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c > index f49d1a36d3ae..f62de5f43e79 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -911,7 +911,26 @@ static notrace void trace_event_raw_event_synth(void *__data, > strscpy(str_field, str_val, STR_VAR_LEN_MAX); > n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > } else { > - entry->fields[n_u64] = var_ref_vals[var_ref_idx + i]; > + struct synth_field *field = event->fields[i]; > + u64 val = var_ref_vals[var_ref_idx + i]; > + > + switch (field->size) { > + case 1: > + *(u8 *)&entry->fields[n_u64] = (u8)val; > + break; > + > + case 2: > + *(u16 *)&entry->fields[n_u64] = (u16)val; > + break; > + > + case 4: > + *(u32 *)&entry->fields[n_u64] = (u32)val; > + break; > + > + default: > + entry->fields[n_u64] = val; > + break; > + } > n_u64++; > } > }
Hi Steve, On Wed, Dec 18, 2019 at 10:24:23AM -0500, Steven Rostedt wrote: > On Wed, 18 Dec 2019 08:44:27 +0100 > Sven Schnelle <svens@linux.ibm.com> wrote: > > > At least on PA-RISC and s390 synthetic histogram triggers are failing > > selftests because trace_event_raw_event_synth() always writes a 64 bit > > values, but the reader expects a field->size sized value. On little endian > > machines this doesn't hurt, but on big endian this makes the reader always > > read zero values. > > Does this fix make the other one obsolete? > > https://lore.kernel.org/r/20191211110959.2baeb70f@gandalf.local.home No, that's a completely different issue, so we need both. Regards Sven
On Wed, 18 Dec 2019 08:44:27 +0100 Sven Schnelle <svens@linux.ibm.com> wrote: > At least on PA-RISC and s390 synthetic histogram triggers are failing > selftests because trace_event_raw_event_synth() always writes a 64 bit > values, but the reader expects a field->size sized value. On little endian > machines this doesn't hurt, but on big endian this makes the reader always > read zero values. Tom, Does this patch look fine to you? Also, it was only sent to linux-trace-devel. You can see the original patch here: https://lore.kernel.org/linux-trace-devel/20191218074427.96184-4-svens@linux.ibm.com/ -- Steve > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > --- > kernel/trace/trace_events_hist.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c > index f49d1a36d3ae..f62de5f43e79 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -911,7 +911,26 @@ static notrace void trace_event_raw_event_synth(void *__data, > strscpy(str_field, str_val, STR_VAR_LEN_MAX); > n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > } else { > - entry->fields[n_u64] = var_ref_vals[var_ref_idx + i]; > + struct synth_field *field = event->fields[i]; > + u64 val = var_ref_vals[var_ref_idx + i]; > + > + switch (field->size) { > + case 1: > + *(u8 *)&entry->fields[n_u64] = (u8)val; > + break; > + > + case 2: > + *(u16 *)&entry->fields[n_u64] = (u16)val; > + break; > + > + case 4: > + *(u32 *)&entry->fields[n_u64] = (u32)val; > + break; > + > + default: > + entry->fields[n_u64] = val; > + break; > + } > n_u64++; > } > }
Hi Steve, On Thu, 2019-12-19 at 18:33 -0500, Steven Rostedt wrote: > On Wed, 18 Dec 2019 08:44:27 +0100 > Sven Schnelle <svens@linux.ibm.com> wrote: > > > At least on PA-RISC and s390 synthetic histogram triggers are > > failing > > selftests because trace_event_raw_event_synth() always writes a 64 > > bit > > values, but the reader expects a field->size sized value. On little > > endian > > machines this doesn't hurt, but on big endian this makes the reader > > always > > read zero values. > > Tom, > > Does this patch look fine to you? Yeah, it looks fine to me. Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com> > > Also, it was only sent to linux-trace-devel. You can see the original > patch here: > > https://lore.kernel.org/linux-trace-devel/20191218074427.96184-4-sv > ens@linux.ibm.com/ > > -- Steve > > > > > > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > > --- > > kernel/trace/trace_events_hist.c | 21 ++++++++++++++++++++- > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_events_hist.c > > b/kernel/trace/trace_events_hist.c > > index f49d1a36d3ae..f62de5f43e79 100644 > > --- a/kernel/trace/trace_events_hist.c > > +++ b/kernel/trace/trace_events_hist.c > > @@ -911,7 +911,26 @@ static notrace void > > trace_event_raw_event_synth(void *__data, > > strscpy(str_field, str_val, > > STR_VAR_LEN_MAX); > > n_u64 += STR_VAR_LEN_MAX / sizeof(u64); > > } else { > > - entry->fields[n_u64] = > > var_ref_vals[var_ref_idx + i]; > > + struct synth_field *field = event- > > >fields[i]; > > + u64 val = var_ref_vals[var_ref_idx + i]; > > + > > + switch (field->size) { > > + case 1: > > + *(u8 *)&entry->fields[n_u64] = > > (u8)val; > > + break; > > + > > + case 2: > > + *(u16 *)&entry->fields[n_u64] = > > (u16)val; > > + break; > > + > > + case 4: > > + *(u32 *)&entry->fields[n_u64] = > > (u32)val; > > + break; > > + > > + default: > > + entry->fields[n_u64] = val; > > + break; > > + } > > n_u64++; > > } > > } > >
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index f49d1a36d3ae..f62de5f43e79 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -911,7 +911,26 @@ static notrace void trace_event_raw_event_synth(void *__data, strscpy(str_field, str_val, STR_VAR_LEN_MAX); n_u64 += STR_VAR_LEN_MAX / sizeof(u64); } else { - entry->fields[n_u64] = var_ref_vals[var_ref_idx + i]; + struct synth_field *field = event->fields[i]; + u64 val = var_ref_vals[var_ref_idx + i]; + + switch (field->size) { + case 1: + *(u8 *)&entry->fields[n_u64] = (u8)val; + break; + + case 2: + *(u16 *)&entry->fields[n_u64] = (u16)val; + break; + + case 4: + *(u32 *)&entry->fields[n_u64] = (u32)val; + break; + + default: + entry->fields[n_u64] = val; + break; + } n_u64++; } }
At least on PA-RISC and s390 synthetic histogram triggers are failing selftests because trace_event_raw_event_synth() always writes a 64 bit values, but the reader expects a field->size sized value. On little endian machines this doesn't hurt, but on big endian this makes the reader always read zero values. Signed-off-by: Sven Schnelle <svens@linux.ibm.com> --- kernel/trace/trace_events_hist.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)