Re: [PATCH 2/5] ipc/sem: rework task wakeups

[Davidlohr Bueso]

On Tue, 13 Sep 2016, Manfred Spraul wrote:

> > +	if ((error = queue.status) != -EINTR && !signal_pending(current)) {
> > +		/*
> > +		 * User space could assume that semop() is a memory barrier:
> > +		 * Without the mb(), the cpu could speculatively read in user
> > +		 * space stale data that was overwritten by the previous owner
> > +		 * of the semaphore.
> >  		 */
> >  		smp_mb();
> > -
> >  		goto out_free;
> >  	}
> >  	rcu_read_lock();
> >  	sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum);

> What is the purpose of the !signal_pending()?
> Even if there is a signal: If someone has set queue.status, then our 
> semaphore operations completed and we must return that result code.

It was a way of detecting being awoken by an unrelated event while the task
is marked for wakeup and wake_up_process is still not called and force the
slowpath. The same window that get_queue_result deals with by busy waiting
when IN_WAKEUP.

> Obviously: At syscall return, the syscall return code will notice the 
> pending signal and immediately the signal handler is called, but I 
> don't see that this prevents us from using the fast path.

Right, and we cannot race with sys_exit. Will drop the signal check.

> And, at least my opinion: I would avoid placing the error= into the if().

Sure, agreed.

Thanks,
Davidlohr