[lkml]   [2008]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] Replace completions with semaphores
Linus Torvalds wrote:
> On Tue, 15 Apr 2008, Andi Kleen wrote:
>>> - probably add support for completions to do counting
>> But that's just a semaphore, isn't it?
> Exactly. But the point here is:
> - nobody should use semaphores anyway (use mutexes)

For normal locks. But if you have N number of outstanding
events you need to wait for the semaphore is the right primitive.

And it seems there is a not high but non trivial number
of places in the kernel who have a legitimate need for this.

> - making *more* code use semaphores is wrong
> - completions have a different _mental_ model
> IOW, this is not about implementation issues. It's about how you think
> about the operations.

Ok so you just want to rename it.

Fine for me. I always found up() and down() unintuitive anyways
(but it's admittedly better than "P" and "V" which some other systems use)

> We should _not_ implement completions as semaphores, simply because we
> want to get *rid* of semaphores some day.
> So rather than this long and involved patch series that first makes
> semaphores generic, and then makes them be used as completions, I'd much
> rather just skip this whole pointless exercise entirely.
> Why have "generic semaphores" at all, if we want to get rid of them?

Because we still "counted completions" for some things and that's the
same code?

Rather i suspect the real problem is not the name, but just not sure
it gets abused. That is largely more a review problem and as far as I
can figure out basically all the usual reviewers take care of that
anyways. But renaming it also probably wouldn't hurt.

[IMHO I always thought we should have a maintained single "list of
things for reviewers to watch out for" list somewhere]


 \ /
  Last update: 2008-04-15 19:17    [W:0.113 / U:52.568 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site