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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100921143555.GA2873@redhat.com>
Date:	Tue, 21 Sep 2010 10:35:55 -0400
From:	Jason Baron <jbaron@...hat.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu,
	mathieu.desnoyers@...ymtl.ca, hpa@...or.com, tglx@...utronix.de,
	rostedt@...dmis.org, roland@...hat.com, rth@...hat.com,
	mhiramat@...hat.com, fweisbec@...il.com, avi@...hat.com,
	davem@...emloft.net, vgoyal@...hat.com, sam@...nborg.org,
	tony@...eyournoodle.com
Subject: Re: [PATCH 03/10] jump label v11: base patch

On Tue, Sep 21, 2010 at 03:12:32PM +0200, Andi Kleen wrote:
> 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?
> 

So far, yes. But I didn't want to force this, in case the users of the
API wanted to use other types. But I'm ok with 'int *' here.

> > +#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

So there are ~150 tracepoints, but this code is also being proposed for
use with 'dynamic debug' of which there are > 1000, and I'm hoping for
more users moving forward.

Also, I think the hash table deals nicely with modules. I create a
linked list of only those module sections that are relevant to each hash
bucket. If you search through all the section on each enable/disable,
its going to be proportional to the number of modules as well.

thanks,

-Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/