[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231201091007.GG3818@noisy.programming.kicks-ass.net>
Date: Fri, 1 Dec 2023 10:10:07 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Jann Horn <jannh@...gle.com>
Cc: Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
Waiman Long <longman@...hat.com>,
Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH] locking: Document that mutex_unlock() is non-atomic
On Thu, Nov 30, 2023 at 09:48:17PM +0100, Jann Horn wrote:
> I have seen several cases of attempts to use mutex_unlock() to release an
> object such that the object can then be freed by another task.
> My understanding is that this is not safe because mutex_unlock(), in the
> MUTEX_FLAG_WAITERS && !MUTEX_FLAG_HANDOFF case, accesses the mutex
> structure after having marked it as unlocked; so mutex_unlock() requires
> its caller to ensure that the mutex stays alive until mutex_unlock()
> returns.
>
> If MUTEX_FLAG_WAITERS is set and there are real waiters, those waiters
> have to keep the mutex alive, I think; but we could have a spurious
> MUTEX_FLAG_WAITERS left if an interruptible/killable waiter bailed
> between the points where __mutex_unlock_slowpath() did the cmpxchg
> reading the flags and where it acquired the wait_lock.
>
> (With spinlocks, that kind of code pattern is allowed and, from what I
> remember, used in several places in the kernel.)
>
> If my understanding of this is correct, we should probably document this -
> I think such a semantic difference between mutexes and spinlocks is fairly
> unintuitive.
IIRC this is true of all sleeping locks, and I think completion was the
explcicit exception here, but it's been a while.
> index 78540cd7f54b..087716bfa7b2 100644
> --- a/Documentation/locking/mutex-design.rst
> +++ b/Documentation/locking/mutex-design.rst
> @@ -101,6 +101,12 @@ features that make lock debugging easier and faster:
> - Detects multi-task circular deadlocks and prints out all affected
> locks and tasks (and only those tasks).
>
> +Releasing a mutex is not an atomic operation: Once a mutex release operation
Well, it very much is an atomic store-release. That is, I object to your
confusing use of atomic here :-)
Powered by blists - more mailing lists