Messages in this thread | | | Date | Tue, 10 Jun 2014 20:08:37 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: safety of *mutex_unlock() (Was: [BUG] signal: sighand unprotected when accessed by /proc) |
| |
On Tue, 10 Jun 2014, Oleg Nesterov wrote:
> On 06/10, Thomas Gleixner wrote: > > > > On Mon, 9 Jun 2014, Steven Rostedt wrote: > > > I think rtmutex has an > > > issue with it too. Specifically in the slow_unlock case: > > > > > > if (!rt_mutex_has_waiters(lock)) { > > > lock->owner = NULL; > > > raw_spin_unlock(&lock->wait_lock); > > > return; > > > } > > > > Indeed. If the fast path is enabled we have that issue. Fortunately > > there is a halfways reasonable solution for this. > > Ah, yes, I missed that, > > > + while (!rt_mutex_has_waiters(lock)) { > > + /* Drops lock->wait_lock ! */ > > + if (unlock_rt_mutex_safe(lock) == true) > > + return; > > + /* Relock the rtmutex and try again */ > > + raw_spin_lock(&lock->wait_lock); > > } > > OK... > > wakeup_next_waiter() does rt_mutex_set_owner(NULL) before we drop ->wait_lock, > but this looks fine: we know that rt_mutex_has_waiters() can not become false > until waiter->task takes this lock and does rt_mutex_dequeue(), so ->owner > can't be NULL, right?
Correct.
> Perhaps it could simply do ->owner = RT_MUTEX_HAS_WAITERS to make this more > clear...
Good point. The new owner can cleanup the mess.
> Off-topic question. I simply can't understand why rt_mutex_slowtrylock() checks > rt_mutex_owner(lock) != current. This looks pointless, try_to_take_rt_mutex() > always fails (correctly) if rt_mutex_owner() != NULL ? IOW, can't we simply > remove this check or turn it into "if (!rt_mutex_owner(lock))" ?
Indeed.
Thanks,
tglx
| |