Re: [PATCH 11/12] Generic semaphore implementation

From: Harvey Harrison
Date: Thu Feb 28 2008 - 21:38:18 EST


> +/*
> + * Because this function is inlined, the 'state' parameter will be constant,
> + * and thus optimised away by the compiler.
> + */

If so, is unlikely() still needed?

> +static inline int __sched __down_common(struct semaphore *sem, long state)
> +{
> + int result = 0;
> + struct task_struct *task = current;
> + struct semaphore_waiter waiter;
> +
> + list_add_tail(&waiter.list, &sem->wait_list);
> + waiter.task = task;
> + waiter.up = 0;
> +
> + for (;;) {
> + if (unlikely((state == TASK_INTERRUPTIBLE &&
> + signal_pending(task)) ||
> + (state == TASK_KILLABLE &&
> + fatal_signal_pending(task))))
> + goto interrupted;

if (state == TASK_INTERRUPTIBLE && signal_pending(task))
goto interrupted;
if (state == TASK_KILLABLE && fatal_signal_pending(task))
goto interrupted;

Other than that small bit, looks great, any thoughts to getting rid of
all the likely/unlikely as well? If they're no longer performance
critical, it would be nice to have it that little bit cleaner.


Harvey

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