linux-kernel - Re: [PATCH/RFC 1/2] jump label: make enable/disable o(1)

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]

Date:	Thu, 16 Dec 2010 14:23:03 -0500
From:	Jason Baron <jbaron@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	hpa@...or.com, rostedt@...dmis.org, mingo@...e.hu,
	mathieu.desnoyers@...ymtl.ca, tglx@...utronix.de,
	andi@...stfloor.org, roland@...hat.com, rth@...hat.com,
	masami.hiramatsu.pt@...achi.com, fweisbec@...il.com,
	avi@...hat.com, davem@...emloft.net, sam@...nborg.org,
	ddaney@...iumnetworks.com, michael@...erman.id.au,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH/RFC 1/2] jump label: make enable/disable o(1)

On Thu, Dec 16, 2010 at 08:10:02PM +0100, Peter Zijlstra wrote:
> On Thu, 2010-12-16 at 13:25 -0500, Jason Baron wrote:
> > Previously, I allowed any variable type to be used as the 'key' for
> > the jump label. However, by enforcing a type, we can make use of the
> > contents of the 'key'. This patch thus introduces:
> > 
> > struct jump_label_key {
> >        void *ptr;
> > };
> > 
> > The 'ptr' is used a pointer into the jump label table of the
> > corresponding addresses that need to be updated. Thus, when jump labels
> > are enabled/disabled we have a constant time algorithm. There is no
> > longer any hashing.
> > 
> > When jump lables are disabled we simply have:
> > 
> > struct jump_label_key {
> >         int state;
> > };
> > 
> > I've also defined an analogous structure for ref counted jump labels as
> > per a request from Peter.
> > 
> > struct jump_label_keyref {
> >        void *ptr;
> > };
> > 
> > And for the jump labels disabled case:
> > 
> > 
> > struct jump_label_keyref {
> >         atomic_t refcount;
> > };
> > 
> > The reason I've introduced an additional structure for the reference counted
> > jump labels is twofold:
> > 
> > 1) For the jump labels disabled case, reference counted jump labels use an
> > atomic_read(). I didn't want to impact the jump labels disabled case for
> > tracepoints which simply accesses an 'int'.
> > 
> > 2) By introducing a second type, we have two parallel APIs:
> > 
> > extern void jump_label_enable(struct jump_label_key *key);
> > extern void jump_label_disable(struct jump_label_key *key);
> > 
> > static inline void jump_label_inc(struct jump_label_keyref *key)
> > static inline void jump_label_dec(struct jump_label_keyref *key)
> > 
> > In this way, we can't mix up the reference counted API, with the straight
> > enable/disable API since they accept different types.
> 
> But why do we want to have two APIs?

its not 2 APIS doing the same thing...one does refcounting, the other is
a straight enable/disable.

For the jump label disabled case, perf is using atomic_inc/dec and atomic_read
to check if enabled. While other consumers (tracepoints) are just using an
'int'. I didn't want hurt the jump label disabled case for tracepoints.
If we can agree to use atomic ops for tracepoints, or drop atomics from
perf, that would simplify things.

For the jump albel enabled case, there is no issue.

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/