Re: [PATCH V2] trace-cmd: add option to group like comms for profile

From: Steven Rostedt
Date: Wed Jun 24 2015 - 23:07:34 EST


On Thu, 21 May 2015 13:30:08 -0400
Josef Bacik <jbacik@xxxxxx> wrote:


> V1->V2:
> -renamed the option to --by-comm, added it to trace-cmd report --profile as well
> -fixed up the string hash

Or break it ;-)

> -changed it to merge all events after the fact so it's less error prone

> diff --git a/trace-hash-local.h b/trace-hash-local.h
> index 997b11c..eaeeaaf 100644
> --- a/trace-hash-local.h
> +++ b/trace-hash-local.h
> @@ -48,4 +48,13 @@ static inline unsigned int trace_hash(int val)
> return hash;
> }
>
> +static inline unsigned int trace_hash_str(char *str)
> +{
> + int val = 0;
> + int i;
> +
> + for (i = 0; str[i]; i++)
> + val += ((int)str[i]) << (i & 0xffffff);
> + return trace_hash(val);
> +}
>

I need to clean out my medicine cabinet and remove all the expired
meds. Because I was definitely taking something nasty when I
recommended (i & 0xffffff)!

When i is greater than 32 (which is less than 0xffffff) it will
overflow the addition. What I wanted was that we don't shift more than
24 bits. Where 2 ** 24 - 1 == 0xffffff.

That should be:

val += ((int)str[i]) << (i % 25);

my bad :-/


To avoid the slow '%', I'll just use '& 0xf', as shifting it 16 times is
enough for this algorithm.

No need to send a new patch, I'll fix it, as it was my brain
fart. I'll review the rest of this patch too and apply it if nothing
sticks out.

Thanks!

-- Steve

--
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/