* Re: ftrace histogram sorting broken on BE architecures [not found] <20191211123316.GD12147@stackframe.org> @ 2019-12-11 15:35 ` Steven Rostedt 2019-12-11 16:09 ` Steven Rostedt 0 siblings, 1 reply; 15+ messages in thread From: Steven Rostedt @ 2019-12-11 15:35 UTC (permalink / raw) To: Sven Schnelle; +Cc: linux-trace-devel, LKML, Tom Zanussi On Wed, 11 Dec 2019 13:33:16 +0100 Sven Schnelle <svens@stackframe.org> wrote: > Hi List, Hi Sven, > > i was looking into a ftracetest failure on s390: > > # ./ftracetest test.d/trigger/trigger-hist.tc > === Ftrace unit tests === > [1] event trigger - test histogram trigger [FAIL] > [2] (instance) event trigger - test histogram trigger [FAIL] > > from the -vvv log: ++ fail 'sort param on sched_process_fork did not work' > > # cat events/sched/sched_process_fork/hist > > # event histogram > # > # trigger info: hist:keys=parent_pid,child_pid:vals=hitcount:sort=child_pid:size=2048 [active] > # > > { parent_pid: 1406, child_pid: 1428 } hitcount: 1 > { parent_pid: 1406, child_pid: 1430 } hitcount: 1 > { parent_pid: 1406, child_pid: 1427 } hitcount: 1 > { parent_pid: 1406, child_pid: 1432 } hitcount: 1 > { parent_pid: 1406, child_pid: 1431 } hitcount: 1 > { parent_pid: 1406, child_pid: 1429 } hitcount: 1 > > So the test is right, the entries are not sorted. After digging into the > ftrace code i noticed that integer values always get extended to 64 bit > in event_hist_trigger(), but cmp_entries_key() from tracing_map.c uses the > type of the field (which is a pid_t, and therefore 4 bytes). > > On Little Endian this doesn't hurt, but on BE s390 this makes the compare > function compare 4 zero bytes, which is the reason why sorting doesn't > work. As a test i forced the compare function used in cmp_entries_key() to > tracing_map_cmp_s64(), which made the ftrace tests pass. > > I also tested this on 64 bit parisc with the same results, so the architecture > doesn't seem make a difference (besides LE vs. BE) > > Any thoughts on how to fix this? I'm not sure whether i fully understand the > ftrace maps... ;-) Your analysis makes sense. I'll take a deeper look at it. Thanks for reporting it! -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 15:35 ` ftrace histogram sorting broken on BE architecures Steven Rostedt @ 2019-12-11 16:09 ` Steven Rostedt 2019-12-11 16:37 ` Tom Zanussi ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Steven Rostedt @ 2019-12-11 16:09 UTC (permalink / raw) To: Sven Schnelle; +Cc: linux-trace-devel, LKML, Tom Zanussi On Wed, 11 Dec 2019 10:35:57 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > > Any thoughts on how to fix this? I'm not sure whether i fully understand the > > ftrace maps... ;-) > > Your analysis makes sense. I'll take a deeper look at it. Sven, Does this patch fix it for you? Tom, Correct me if I'm wrong, from what I can tell, all sums and keys are u64 unless they are a string. Thus, I believe this patch should not have any issues. -- Steve diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c index 9a1c22310323..9e31bfc818ff 100644 --- a/kernel/trace/tracing_map.c +++ b/kernel/trace/tracing_map.c @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, void *val_b) #define DEFINE_TRACING_MAP_CMP_FN(type) \ static int tracing_map_cmp_##type(void *val_a, void *val_b) \ { \ - type a = *(type *)val_a; \ - type b = *(type *)val_b; \ + type a = (type)(*(u64 *)val_a); \ + type b = (type)(*(u64 *)val_b); \ \ return (a > b) ? 1 : ((a < b) ? -1 : 0); \ } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 16:09 ` Steven Rostedt @ 2019-12-11 16:37 ` Tom Zanussi 2019-12-11 17:00 ` Steven Rostedt 2019-12-11 18:14 ` Sven Schnelle 2019-12-12 19:17 ` Tom Zanussi 2 siblings, 1 reply; 15+ messages in thread From: Tom Zanussi @ 2019-12-11 16:37 UTC (permalink / raw) To: Steven Rostedt, Sven Schnelle; +Cc: linux-trace-devel, LKML Hi Steve, Sven, On Wed, 2019-12-11 at 11:09 -0500, Steven Rostedt wrote: > On Wed, 11 Dec 2019 10:35:57 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > Any thoughts on how to fix this? I'm not sure whether i fully > > > understand the > > > ftrace maps... ;-) > > > > Your analysis makes sense. I'll take a deeper look at it. > > Sven, > > Does this patch fix it for you? > > Tom, > > Correct me if I'm wrong, from what I can tell, all sums and keys are > u64 unless they are a string. Thus, I believe this patch should not > have any issues. The sums are u64, but the keys may not be. I'll take a look and see, but I'm out today and won't be able to look into it until tomorrow, if that's ok. Tom > > -- Steve > > diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c > index 9a1c22310323..9e31bfc818ff 100644 > --- a/kernel/trace/tracing_map.c > +++ b/kernel/trace/tracing_map.c > @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, > void *val_b) > #define DEFINE_TRACING_MAP_CMP_FN(type) > \ > static int tracing_map_cmp_##type(void *val_a, void *val_b) > \ > { > \ > - type a = *(type *)val_a; > \ > - type b = *(type *)val_b; > \ > + type a = (type)(*(u64 *)val_a); > \ > + type b = (type)(*(u64 *)val_b); > \ > > \ > return (a > b) ? 1 : ((a < b) ? -1 : 0); > \ > } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 16:37 ` Tom Zanussi @ 2019-12-11 17:00 ` Steven Rostedt 2019-12-11 19:26 ` Tom Zanussi 0 siblings, 1 reply; 15+ messages in thread From: Steven Rostedt @ 2019-12-11 17:00 UTC (permalink / raw) To: Tom Zanussi; +Cc: Sven Schnelle, linux-trace-devel, LKML On Wed, 11 Dec 2019 10:37:18 -0600 Tom Zanussi <zanussi@kernel.org> wrote: > > Tom, > > > > Correct me if I'm wrong, from what I can tell, all sums and keys are > > u64 unless they are a string. Thus, I believe this patch should not > > have any issues. > > The sums are u64, but the keys may not be. I'll take a look and see, Are they? I see in create_key_field: key_size = ALIGN(key_size, sizeof(u64)); Which to me seems that we'll have nothing smaller than sizeof(u64). > but I'm out today and won't be able to look into it until tomorrow, if > that's ok. No rush. I'll start the testing of this patch if you come back and say its fine ;-) That takes a full day. -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 17:00 ` Steven Rostedt @ 2019-12-11 19:26 ` Tom Zanussi 2019-12-12 18:07 ` Steven Rostedt 0 siblings, 1 reply; 15+ messages in thread From: Tom Zanussi @ 2019-12-11 19:26 UTC (permalink / raw) To: Steven Rostedt; +Cc: Sven Schnelle, linux-trace-devel, LKML Hi Steve, On Wed, 2019-12-11 at 12:00 -0500, Steven Rostedt wrote: > On Wed, 11 Dec 2019 10:37:18 -0600 > Tom Zanussi <zanussi@kernel.org> wrote: > > > > Tom, > > > > > > Correct me if I'm wrong, from what I can tell, all sums and keys > > > are > > > u64 unless they are a string. Thus, I believe this patch should > > > not > > > have any issues. > > > > The sums are u64, but the keys may not be. I'll take a look and > > see, > > Are they? I see in create_key_field: > > key_size = ALIGN(key_size, sizeof(u64)); > > Which to me seems that we'll have nothing smaller than sizeof(u64). > Yeah, that makes them effectively u64 in this case, so I'd agree. Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 19:26 ` Tom Zanussi @ 2019-12-12 18:07 ` Steven Rostedt 2019-12-12 19:15 ` Tom Zanussi 0 siblings, 1 reply; 15+ messages in thread From: Steven Rostedt @ 2019-12-12 18:07 UTC (permalink / raw) To: Tom Zanussi; +Cc: Sven Schnelle, linux-trace-devel, LKML On Wed, 11 Dec 2019 13:26:03 -0600 Tom Zanussi <zanussi@kernel.org> wrote: > > Are they? I see in create_key_field: > > > > key_size = ALIGN(key_size, sizeof(u64)); > > > > Which to me seems that we'll have nothing smaller than sizeof(u64). > > > > Yeah, that makes them effectively u64 in this case, so I'd agree. Hi Tom, Does this mean it's good to go? It just passed my test suite, and I can send it to Linus now. -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-12 18:07 ` Steven Rostedt @ 2019-12-12 19:15 ` Tom Zanussi 0 siblings, 0 replies; 15+ messages in thread From: Tom Zanussi @ 2019-12-12 19:15 UTC (permalink / raw) To: Steven Rostedt; +Cc: Sven Schnelle, linux-trace-devel, LKML Hi Steve, On Thu, 2019-12-12 at 13:07 -0500, Steven Rostedt wrote: > On Wed, 11 Dec 2019 13:26:03 -0600 > Tom Zanussi <zanussi@kernel.org> wrote: > > > > Are they? I see in create_key_field: > > > > > > key_size = ALIGN(key_size, sizeof(u64)); > > > > > > Which to me seems that we'll have nothing smaller than > > > sizeof(u64). > > > > > > > Yeah, that makes them effectively u64 in this case, so I'd agree. > > Hi Tom, > > Does this mean it's good to go? It just passed my test suite, and I > can > send it to Linus now. > Yes, I think so. I'll go and ack the patch. Thanks, Tom > -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 16:09 ` Steven Rostedt 2019-12-11 16:37 ` Tom Zanussi @ 2019-12-11 18:14 ` Sven Schnelle 2019-12-12 19:17 ` Tom Zanussi 2 siblings, 0 replies; 15+ messages in thread From: Sven Schnelle @ 2019-12-11 18:14 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, LKML, Tom Zanussi Hi Steven, On Wed, Dec 11, 2019 at 11:09:59AM -0500, Steven Rostedt wrote: > On Wed, 11 Dec 2019 10:35:57 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > Any thoughts on how to fix this? I'm not sure whether i fully understand the > > > ftrace maps... ;-) > > > > Your analysis makes sense. I'll take a deeper look at it. > > Does this patch fix it for you? Yes, it does. Thanks for looking into this. > Correct me if I'm wrong, from what I can tell, all sums and keys are > u64 unless they are a string. Thus, I believe this patch should not > have any issues. I'll retest if Tom comes up with another patch. Thanks, Sven ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-11 16:09 ` Steven Rostedt 2019-12-11 16:37 ` Tom Zanussi 2019-12-11 18:14 ` Sven Schnelle @ 2019-12-12 19:17 ` Tom Zanussi 2019-12-16 15:47 ` David Laight 2 siblings, 1 reply; 15+ messages in thread From: Tom Zanussi @ 2019-12-12 19:17 UTC (permalink / raw) To: Steven Rostedt, Sven Schnelle; +Cc: linux-trace-devel, LKML On Wed, 2019-12-11 at 11:09 -0500, Steven Rostedt wrote: > On Wed, 11 Dec 2019 10:35:57 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > Any thoughts on how to fix this? I'm not sure whether i fully > > > understand the > > > ftrace maps... ;-) > > > > Your analysis makes sense. I'll take a deeper look at it. > > Sven, > > Does this patch fix it for you? > > Tom, > > Correct me if I'm wrong, from what I can tell, all sums and keys are > u64 unless they are a string. Thus, I believe this patch should not > have any issues. > > -- Steve > > diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c > index 9a1c22310323..9e31bfc818ff 100644 > --- a/kernel/trace/tracing_map.c > +++ b/kernel/trace/tracing_map.c > @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, > void *val_b) > #define DEFINE_TRACING_MAP_CMP_FN(type) > \ > static int tracing_map_cmp_##type(void *val_a, void *val_b) > \ > { > \ > - type a = *(type *)val_a; > \ > - type b = *(type *)val_b; > \ > + type a = (type)(*(u64 *)val_a); > \ > + type b = (type)(*(u64 *)val_b); > \ > > \ > return (a > b) ? 1 : ((a < b) ? -1 : 0); > \ > } Acked-by: Tom Zanussi <zanussi@kernel.org> ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: ftrace histogram sorting broken on BE architecures 2019-12-12 19:17 ` Tom Zanussi @ 2019-12-16 15:47 ` David Laight 2019-12-16 16:05 ` Steven Rostedt 0 siblings, 1 reply; 15+ messages in thread From: David Laight @ 2019-12-16 15:47 UTC (permalink / raw) To: 'Tom Zanussi', Steven Rostedt, Sven Schnelle Cc: linux-trace-devel, LKML > From: Tom Zanussi > Sent: 12 December 2019 19:17 > On Wed, 2019-12-11 at 11:09 -0500, Steven Rostedt wrote: > > On Wed, 11 Dec 2019 10:35:57 -0500 > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > Any thoughts on how to fix this? I'm not sure whether i fully > > > > understand the > > > > ftrace maps... ;-) > > > > > > Your analysis makes sense. I'll take a deeper look at it. > > > > Sven, > > > > Does this patch fix it for you? > > > > Tom, > > > > Correct me if I'm wrong, from what I can tell, all sums and keys are > > u64 unless they are a string. Thus, I believe this patch should not > > have any issues. ... > > --- a/kernel/trace/tracing_map.c > > +++ b/kernel/trace/tracing_map.c > > @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, > > void *val_b) > > #define DEFINE_TRACING_MAP_CMP_FN(type) \ > > static int tracing_map_cmp_##type(void *val_a, void *val_b) \ > > { \ > > - type a = *(type *)val_a; \ > > - type b = *(type *)val_b; \ > > + type a = (type)(*(u64 *)val_a); \ > > + type b = (type)(*(u64 *)val_b); \ > > \ > > return (a > b) ? 1 : ((a < b) ? -1 : 0); \ > > } That looks so horrid/wrong it can't be right on both BE and LE. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-16 15:47 ` David Laight @ 2019-12-16 16:05 ` Steven Rostedt 2019-12-16 17:06 ` David Laight 0 siblings, 1 reply; 15+ messages in thread From: Steven Rostedt @ 2019-12-16 16:05 UTC (permalink / raw) To: David Laight Cc: 'Tom Zanussi', Sven Schnelle, linux-trace-devel, LKML On Mon, 16 Dec 2019 15:47:12 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > > From: Tom Zanussi > > Sent: 12 December 2019 19:17 > > On Wed, 2019-12-11 at 11:09 -0500, Steven Rostedt wrote: > > > On Wed, 11 Dec 2019 10:35:57 -0500 > > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > > > Any thoughts on how to fix this? I'm not sure whether i fully > > > > > understand the > > > > > ftrace maps... ;-) > > > > > > > > Your analysis makes sense. I'll take a deeper look at it. > > > > > > Sven, > > > > > > Does this patch fix it for you? > > > > > > Tom, > > > > > > Correct me if I'm wrong, from what I can tell, all sums and keys are > > > u64 unless they are a string. Thus, I believe this patch should not > > > have any issues. > ... > > > --- a/kernel/trace/tracing_map.c > > > +++ b/kernel/trace/tracing_map.c > > > @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, > > > void *val_b) > > > #define DEFINE_TRACING_MAP_CMP_FN(type) \ > > > static int tracing_map_cmp_##type(void *val_a, void *val_b) \ > > > { \ > > > - type a = *(type *)val_a; \ > > > - type b = *(type *)val_b; \ > > > + type a = (type)(*(u64 *)val_a); \ > > > + type b = (type)(*(u64 *)val_b); \ > > > \ > > > return (a > b) ? 1 : ((a < b) ? -1 : 0); \ > > > } > > That looks so horrid/wrong it can't be right on both BE and LE. Well, the original is obviously not right for both BE and LE, but the fix is: type a = (type)(*(u64 *)val_a); Which breaks down to: (u64 *)val_a - make val_a a pointer to a u64 number all values were written as u64. u64 data = (u64)original_val_a Where original_val_a could be a byte, short, int, long or long long. Now that we have (u64 *)val_a, we dereference it: *(u64 *)val_a - which gives us a u64 number. This u64 number should be the same as the u64 data. Then we convert this as: (type)(*(u64 *)val_a) Taking the u64 number we retrieved and converted it back to the original type that was recorded. In other words, if the following should work: type a = x; u64 data = (u64)a; u64 *ptr = &data; u64 b_data = *ptr; type b = (type)b_data; If b == a above, then there should be nothing wrong with this patch. The compiler should know how to map those type conversions properly for both BE and LE. -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: ftrace histogram sorting broken on BE architecures 2019-12-16 16:05 ` Steven Rostedt @ 2019-12-16 17:06 ` David Laight 2019-12-16 18:29 ` Steven Rostedt 0 siblings, 1 reply; 15+ messages in thread From: David Laight @ 2019-12-16 17:06 UTC (permalink / raw) To: 'Steven Rostedt' Cc: 'Tom Zanussi', Sven Schnelle, linux-trace-devel, LKML From: Steven Rostedt > Sent: 16 December 2019 16:06 > On Mon, 16 Dec 2019 15:47:12 +0000 > David Laight <David.Laight@ACULAB.COM> wrote: > > > > From: Tom Zanussi > > > Sent: 12 December 2019 19:17 > > > On Wed, 2019-12-11 at 11:09 -0500, Steven Rostedt wrote: > > > > On Wed, 11 Dec 2019 10:35:57 -0500 > > > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > > > > > Any thoughts on how to fix this? I'm not sure whether i fully > > > > > > understand the > > > > > > ftrace maps... ;-) > > > > > > > > > > Your analysis makes sense. I'll take a deeper look at it. > > > > > > > > Sven, > > > > > > > > Does this patch fix it for you? > > > > > > > > Tom, > > > > > > > > Correct me if I'm wrong, from what I can tell, all sums and keys are > > > > u64 unless they are a string. Thus, I believe this patch should not > > > > have any issues. > > ... > > > > --- a/kernel/trace/tracing_map.c > > > > +++ b/kernel/trace/tracing_map.c > > > > @@ -148,8 +148,8 @@ static int tracing_map_cmp_atomic64(void *val_a, > > > > void *val_b) > > > > #define DEFINE_TRACING_MAP_CMP_FN(type) \ > > > > static int tracing_map_cmp_##type(void *val_a, void *val_b) \ > > > > { \ > > > > - type a = *(type *)val_a; \ > > > > - type b = *(type *)val_b; \ > > > > + type a = (type)(*(u64 *)val_a); \ > > > > + type b = (type)(*(u64 *)val_b); \ > > > > \ > > > > return (a > b) ? 1 : ((a < b) ? -1 : 0); \ > > > > } > > > > That looks so horrid/wrong it can't be right on both BE and LE. > > Well, the original is obviously not right for both BE and LE, but the > fix is: > > type a = (type)(*(u64 *)val_a); > > Which breaks down to: > > (u64 *)val_a - make val_a a pointer to a u64 number > > all values were written as u64. > > u64 data = (u64)original_val_a > > Where original_val_a could be a byte, short, int, long or long long. I'd sort of guessed that, but then the pointer type passed to tracing_map_cmp_##type() will always be 'u64 *' (since the field the address is taken of must be that type). Then the (u64 *) casts are no longer needed. Possibly you can just pass the u64 values to: tracing_map_cmp_##type(type a, type b) { return a > b ? 1 : a < b ? -1 : 0; } The high bit masking and sign extension is then implicit in the call. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-16 17:06 ` David Laight @ 2019-12-16 18:29 ` Steven Rostedt 2019-12-17 10:05 ` David Laight 0 siblings, 1 reply; 15+ messages in thread From: Steven Rostedt @ 2019-12-16 18:29 UTC (permalink / raw) To: David Laight Cc: 'Tom Zanussi', Sven Schnelle, linux-trace-devel, LKML On Mon, 16 Dec 2019 17:06:50 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > > Where original_val_a could be a byte, short, int, long or long long. > > I'd sort of guessed that, but then the pointer type passed to tracing_map_cmp_##type() > will always be 'u64 *' (since the field the address is taken of must be that type). > Then the (u64 *) casts are no longer needed. > > Possibly you can just pass the u64 values to: > tracing_map_cmp_##type(type a, type b) > { > return a > b ? 1 : a < b ? -1 : 0; > } > > The high bit masking and sign extension is then implicit in the call. But these are used to pass into a compare function that takes compare functions that are something other than numbers. They can be pointers to strings. -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: ftrace histogram sorting broken on BE architecures 2019-12-16 18:29 ` Steven Rostedt @ 2019-12-17 10:05 ` David Laight 2019-12-18 15:33 ` Steven Rostedt 0 siblings, 1 reply; 15+ messages in thread From: David Laight @ 2019-12-17 10:05 UTC (permalink / raw) To: 'Steven Rostedt' Cc: 'Tom Zanussi', Sven Schnelle, linux-trace-devel, LKML From: Steven Rostedt > Sent: 16 December 2019 18:29 > On Mon, 16 Dec 2019 17:06:50 +0000 > David Laight <David.Laight@ACULAB.COM> wrote: > > > > Where original_val_a could be a byte, short, int, long or long long. > > > > I'd sort of guessed that, but then the pointer type passed to tracing_map_cmp_##type() > > will always be 'u64 *' (since the field the address is taken of must be that type). > > Then the (u64 *) casts are no longer needed. > > > > Possibly you can just pass the u64 values to: > > tracing_map_cmp_##type(type a, type b) > > { > > return a > b ? 1 : a < b ? -1 : 0; > > } > > > > The high bit masking and sign extension is then implicit in the call. > > But these are used to pass into a compare function that takes compare > functions that are something other than numbers. They can be pointers > to strings. In that case I think I'd embed the u64 inside a structure and pass the structure address to the compare function. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ftrace histogram sorting broken on BE architecures 2019-12-17 10:05 ` David Laight @ 2019-12-18 15:33 ` Steven Rostedt 0 siblings, 0 replies; 15+ messages in thread From: Steven Rostedt @ 2019-12-18 15:33 UTC (permalink / raw) To: David Laight Cc: 'Tom Zanussi', Sven Schnelle, linux-trace-devel, LKML On Tue, 17 Dec 2019 10:05:57 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > From: Steven Rostedt > > Sent: 16 December 2019 18:29 > > On Mon, 16 Dec 2019 17:06:50 +0000 > > David Laight <David.Laight@ACULAB.COM> wrote: > > > > > > Where original_val_a could be a byte, short, int, long or long long. > > > > > > I'd sort of guessed that, but then the pointer type passed to tracing_map_cmp_##type() > > > will always be 'u64 *' (since the field the address is taken of must be that type). > > > Then the (u64 *) casts are no longer needed. > > > > > > Possibly you can just pass the u64 values to: > > > tracing_map_cmp_##type(type a, type b) > > > { > > > return a > b ? 1 : a < b ? -1 : 0; > > > } > > > > > > The high bit masking and sign extension is then implicit in the call. > > > > But these are used to pass into a compare function that takes compare > > functions that are something other than numbers. They can be pointers > > to strings. > > In that case I think I'd embed the u64 inside a structure and pass the structure > address to the compare function. > It's something we can clean up later. As this is going to be marked for stable, and the code still works, I would like to keep the change as simple as possible. Thanks! -- Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-12-18 15:33 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20191211123316.GD12147@stackframe.org> 2019-12-11 15:35 ` ftrace histogram sorting broken on BE architecures Steven Rostedt 2019-12-11 16:09 ` Steven Rostedt 2019-12-11 16:37 ` Tom Zanussi 2019-12-11 17:00 ` Steven Rostedt 2019-12-11 19:26 ` Tom Zanussi 2019-12-12 18:07 ` Steven Rostedt 2019-12-12 19:15 ` Tom Zanussi 2019-12-11 18:14 ` Sven Schnelle 2019-12-12 19:17 ` Tom Zanussi 2019-12-16 15:47 ` David Laight 2019-12-16 16:05 ` Steven Rostedt 2019-12-16 17:06 ` David Laight 2019-12-16 18:29 ` Steven Rostedt 2019-12-17 10:05 ` David Laight 2019-12-18 15:33 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).