Re: [PATCH] Replace completions with semaphores

[Matthew Wilcox]

On Sat, Apr 12, 2008 at 02:24:41PM +0200, Peter Zijlstra wrote:
> On Fri, 2008-04-11 at 15:00 -0600, Matthew Wilcox wrote:
> > So does it make sense to retain the completion as a primitive
> > in Linux? 
> 
> My take on it is the opposite: kill off semaphores and leave the
> completions around.

OK, so all three of you have got the wrong end of the stick.

My point is that the struct semaphore and the struct completion are
essentially the same data structure, operated on in essentially the same
way and having essentially the same purpose.

It would look bloody odd to write (code taken from megasas_mgmt_ioctl_fw() in
drivers/scsi/megaraid/megaraid_sas.c):

        if (wait_for_completion_interruptible(&instance->ioctl_completion)) {
                error = -ERESTARTSYS;
                goto out_kfree_ioc;
        }
        error = megasas_mgmt_fw_ioctl(instance, user_ioc, ioc);
        complete(&instance->ioctl_sem);

What I'm trying to get a feeling for is whether people find it similarly
odd to use semaphores where we currently use completions.  We *used*
to, but I don't find that a compelling reason.


Arnd contacted me off-list and made the very sensible suggestion of:

struct completion {
	struct semaphore sem;
}

That lets us eliminate the duplicate code since all the completion
functions become very thin wrappers around semaphore operations.

I'll note that the semaphore code I hae queued for 2.6.26 is slightly
more efficient than the current implementation of completions because
completions use the generic waitqueue code and thus do an indirect
function call per wakeup.  Of course, there's no reason completions
couldn't use the same technique as my semaphore code ... but then they
would be identical to semaphores ;-)

So I'm going to redo my patch using Arnd's idea because it allows us
to eliminate code without changing the API at all.  We can continue the
discussion about eliminating one or the other API, but my opinion is that
eliminating either is a mistake.  The choice of API indicates how the
author thinks about the code, which is crucial for those reading the code.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
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/