Re: [patch 3/7] epoll keyed wakeups - introduce key-aware wakeupmacros

[Linus Torvalds]

On Fri, 30 Jan 2009, Davide Libenzi wrote:
>
> The following patch introduces new kwake_* macros that accepts an
> extra key parameter to be specified in the wakeup.

I really hate the naming.

> I chose to add an initial 'k' to the original names, instead of adding
> a whole "_key", since the name of some of those macros is becoming
> awfully long. No problem in using the "_key" naming, if others feel it.
> Comments?

That whole "kwake" thing makes me just think mis-spelling, so it does need 
to change. 

But even more I dislike the notion of this being a "key". It's not. It's 
about poll events, nothing more. So renaming it to "_key()" in no way 
helps.

Yes, _internally_ we send that "void *key" around, and then leave it to 
lower levels to agree about how it is used, but at the level _you_ then 
use it, that is no longer the case. When you do a 

	kwake_up_interruptible(&tty->write_wait, POLLOUT);

that has _nothing_ to do with "keys" any more. So the 'k' prefix is wrong 
and really odd-looking, but a '_key' postfix wouldn't be much better 
either. Because when you pass in POLLOUT, you're not using it as a key, 
you are very much using it as a poll-specific thing.

So the naming should match that. I suspect a '_poll' postfix (or a 'poll_' 
prefix would work and make sense.

So apart from that hating, I think the internal implementation and the use 
of the existing 'key' parameter is fairly sane. The only downside is that 
we've now really used up that key thing for something very epoll-specific, 
but I don't see any better use for it, so I guess that's not a big 
downside.

Oh, and numbers, please. How big of a win is this, really? Preferably with 
something that really uses epoll for something real.

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