linux-kernel - Re: [PATCH/RFC 0/2] jump label: simplify API

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 15:18:23 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	Jason Baron <jbaron@...hat.com>, peterz@...radead.org,
	hpa@...or.com, mingo@...e.hu, 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 0/2] jump label: simplify API

On Thu, 2010-12-16 at 14:22 -0500, Mathieu Desnoyers wrote:
> * Jason Baron (jbaron@...hat.com) wrote:
> > Hi,
> > 
> > The first patch uses the storage space of the jump label key address
> > as a pointer into the update table. In this way, we can find all
> > the addresses that need to be updated without hashing.
> > 
> > The second patch introduces:
> > 
> > static __always_inline bool unlikely_switch(struct jump_label_key *key);
> > 
> > instead of the old JUMP_LABEL(key, label) macro.
> > 
> > In this way, jump labels become really easy to use:
> > 
> > Define:
> > 
> > 	struct jump_label_key jump_key;
> > 
> > Can be used as:
> > 
> > 	if (unlikely_switch(&jump_key))
> > 		do unlikely code
> 
> Ah, yes, that's an improvement!
> 
> I'm just wondering about the terminology here. Isn't that more a
> "branch" than a "switch" ?
> 
> I'm concerned about the fact that if we ever want to use the asm goto
> ability to jump to multiple targets (which is closer to a statically
> computed switch than a branch), we might want to reserve "switch" name
> for that rather than the branch.

Good point.

> 
> I wonder if the "if (unlikely_switch(&jump_key))" you propose above is
> the right thing to do. Why does the unlikely_ have to be included in the
> name ? Maybe there is a good reason for it, but it would be nice to have
> it spelled out. We might consider:
> 
> if (unlikely(static_branch(&jump_key)))
>   ...
> 
> instead.


Hmm, I see your point here too.


> For the switch statement, from the top of my head the idea would be to
> get something close to the following:
> 
> static __always_inline
> int static_switch_{3,4,5,6...}(struct jump_label_key *key);
> 
> e.g.:
> 
> static __always_inline
> int static_switch_3(struct jump_label_key *key)
> {
> 	asm goto("1:"
> 		JUMP_LABEL_INITIAL_NOP
> 		".pushsection __switch_table_3,  \"a\" \n\t"
> 		_ASM_PTR "%c0, 1b, %l[l_1], %l[l_2] \n\t"
> 		".popsection \n\t"
> 		: :  "i" (key) : : l_1, l_2 );
> 	return 0;
> l_1:
> 	return 1;
> l_2:
> 	return 2;
> }
> 
> switch(static_switch_3(&switch_key)) {
> 	case 0: .....
> 		break;
> 	case 1: .....
> 		break;
> 	case 2: .....
> 		break;
> }
> 
> (I have not tried to give that to gcc 4.5.x to see how the resulting
> assembly looks like. It would be interesting to see if it handles this
> case well)

Something to think about later (when we need such a thing). But I do
agree, perhaps calling it static_branch() instead, is a better idea.

-- Steve


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