On Sat, 2013-03-02 at 13:24 -0800, Linus Torvalds wrote:
> On Fri, Mar 1, 2013 at 4:16 PM, Davidlohr Bueso <davidlohr.bueso@xxxxxx> wrote:
> > @@ -784,7 +806,7 @@ struct kern_ipc_perm *ipcctl_pre_down(struct ipc_namespace *ns,
> > int err;
> >
> > down_write(&ids->rw_mutex);
> > - ipcp = ipc_lock_check(ids, id);
> > + ipcp = ipc_obtain_object_check(ids, id);
> > if (IS_ERR(ipcp)) {
> > err = PTR_ERR(ipcp);
> > goto out_up;
> > @@ -801,7 +823,7 @@ struct kern_ipc_perm *ipcctl_pre_down(struct ipc_namespace *ns,
> > return ipcp;
> >
> > err = -EPERM;
> > - ipc_unlock(ipcp);
> > + rcu_read_unlock();
> > out_up:
> > up_write(&ids->rw_mutex);
> > return ERR_PTR(err);
>
> Uhhuh. This is very buggy, and I think it's the reason for the later
> bugs that Emmanuel reported.
Yes, quite buggy. I was able to mess up three different machines with
this, and since semaphores aren't the only users of ipcctl_pre_down(),
it could explain the sys_shmctl() call in the trace Emmanuel reported.
>
> In particular, the *non-error* case is buggy, where it in the middle
> of the function does
>
> return ipcp;
>
> for a successful lookup.
>
> It used to return a locked ipcp, now it no longer does. And you didn't
> change any of the callers, which still do the "ipc_unlock()" at the
> end. So all the locking gets completely confused.
>
After updating the callers, [msgctl, semctl, shmctl]_down, to acquire
the lock for IPC_RMID and IPC_SET commands, I'm no longer seeing these
issues - so far on my regular laptop and two big boxes running my Oracle
benchmarks for a few hours. Something like below (yes, I will address
the open coded spin_lock calls):
@@ -1101,16 +1138,20 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
switch(cmd){
case IPC_RMID:
+ spin_lock(&sma->sem_perm.lock);
freeary(ns, ipcp);
goto out_up;
case IPC_SET:
+ spin_lock(&sma->sem_perm.lock);
err = ipc_update_perm(&semid64.sem_perm, ipcp);
if (err)
goto out_unlock;
sma->sem_ctime = get_seconds();
break;
default:
+ rcu_read_unlock();
err = -EINVAL;
+ goto out_up;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|