Re: [PATCH 03/10] jump label v11: base patch

[Andi Kleen <andi@firstfloor.org>]

On Fri, Sep 17, 2010 at 11:09:00AM -0400, Jason Baron wrote:
> +extern void arch_jump_label_transform(struct jump_entry *entry,
> +				 enum jump_label_type type);
> +extern void jump_label_update(unsigned long key, enum jump_label_type type);
> +extern void jump_label_apply_nops(struct module *mod);
> +extern void arch_jump_label_text_poke_early(jump_label_t addr);

These function names are too long.

Also it would be better if the types for the pointers are kept
instead of casting to unsigned long. All the variables
are ints right?

> +#define JUMP_LABEL_HASH_BITS 6
> +#define JUMP_LABEL_TABLE_SIZE (1 << JUMP_LABEL_HASH_BITS)
> +static struct hlist_head jump_label_table[JUMP_LABEL_TABLE_SIZE];

It's not clear to me why this hash table is needed. There should
not be that many trace points, is it that big a problem to simply
walk all the sections when something is changed?

Or maybe the sections could be just sorted and a binary search used
like with exception tables.

I suspect that would simplify a lot of code.

Overall I like the idea, but the current code is too complicated
for the benefit I think.

Can it be put on a diet?

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.