Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Tue, 5 Dec 2023 06:47:26 +0700
From:   Bagas Sanjaya <bagasdotme@gmail.com>
To:     Jann Horn <jannh@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>
Cc:     Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>,
        David Laight <David.Laight@aculab.com>,
        attreyee-muk <tintinm2017@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Documentation <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v3] locking: Document that some lock types must stay
 alive during unlock
Message-ID: <ZW5lDougvHxM_F64@archie.me>
References: <20231204132259.112152-1-jannh@google.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="MqPrBdX8kks83CUm"
Content-Disposition: inline
In-Reply-To: <20231204132259.112152-1-jannh@google.com>
Precedence: bulk


--MqPrBdX8kks83CUm
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 04, 2023 at 02:22:59PM +0100, Jann Horn wrote:
> diff --git a/Documentation/locking/locktypes.rst b/Documentation/locking/=
locktypes.rst
> index 80c914f6eae7..c9a4bcc967ea 100644
> --- a/Documentation/locking/locktypes.rst
> +++ b/Documentation/locking/locktypes.rst
> @@ -95,6 +95,29 @@ rw_semaphores have a special interface which allows no=
n-owner release for
>  readers.
> =20
> =20
> +Releasing and freeing
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> +For some lock types, such as spinlocks, the lock release operation is de=
signed
> +to allow another concurrent task to free the lock as soon as the lock ha=
s been
> +released - in other words, similarly to refcounts, the unlock operation =
will not
> +access the lock object anymore after marking it as unlocked.
> +
> +This behavior is guaranteed for:
> +
> + - spinlock_t (including in PREEMPT_RT kernels, where spinlock_t is
> +   implemented as an rtmutex)
> +
> +There are other lock types where the lock release operation makes no such
> +guarantee and the caller must ensure that the lock is not destroyed befo=
re the
> +unlock operation has returned.
> +Most sleeping locks are in this category.
> +
> +This is the case in particular for (not an exhaustive list):
> +
> + - mutex
> + - rw_semaphore
> +
> +
>  rtmutex
>  =3D=3D=3D=3D=3D=3D=3D
> =20
> diff --git a/Documentation/locking/mutex-design.rst b/Documentation/locki=
ng/mutex-design.rst
> index 78540cd7f54b..bbb4c4d56ed0 100644
> --- a/Documentation/locking/mutex-design.rst
> +++ b/Documentation/locking/mutex-design.rst
> @@ -101,6 +101,8 @@ 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).
> =20
> +The mutex user must ensure that the mutex is not destroyed while a unlock
> +operation is still in progress, see Documentation/locking/locktypes.rst.
> =20
>  Interfaces
>  ----------
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 2deeeca3e71b..fa4834dba407 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -532,6 +532,11 @@ static noinline void __sched __mutex_unlock_slowpath=
(struct mutex *lock, unsigne
>   * This function must not be used in interrupt context. Unlocking
>   * of a not locked mutex is not allowed.
>   *
> + * The caller must ensure that the mutex stays alive until this function=
 has
> + * returned - mutex_unlock() can NOT directly be used to release an obje=
ct such
> + * that another concurrent task can free it.
> + * See Documentation/locking/locktypes.rst.
> + *
>   * This function is similar to (but not equivalent to) up().
>   */
>  void __sched mutex_unlock(struct mutex *lock)
> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> index 2340b6d90ec6..cbc00a269deb 100644
> --- a/kernel/locking/rwsem.c
> +++ b/kernel/locking/rwsem.c
> @@ -1615,6 +1615,11 @@ EXPORT_SYMBOL(down_write_trylock);
> =20
>  /*
>   * release a read lock
> + *
> + * The caller must ensure that the rw_semaphore stays alive until this f=
unction
> + * has returned - up_read() can NOT directly be used to release an objec=
t such
> + * that another concurrent task can free it.
> + * See Documentation/locking/locktypes.rst.
>   */
>  void up_read(struct rw_semaphore *sem)
>  {
> @@ -1625,6 +1630,11 @@ EXPORT_SYMBOL(up_read);
> =20
>  /*
>   * release a write lock
> + *
> + * The caller must ensure that the rw_semaphore stays alive until this f=
unction
> + * has returned - up_write() can NOT directly be used to release an obje=
ct such
> + * that another concurrent task can free it.
> + * See Documentation/locking/locktypes.rst.
>   */
>  void up_write(struct rw_semaphore *sem)
>  {
>=20
> base-commit: 3b47bc037bd44f142ac09848e8d3ecccc726be99

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

--=20
An old man doll... just what I always wanted! - Clara

--MqPrBdX8kks83CUm
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQSSYQ6Cy7oyFNCHrUH2uYlJVVFOowUCZW5lCQAKCRD2uYlJVVFO
o7AGAQCxQDdMoa5qBDqARjHmnVOnNfr5hLFNc01F/umPrzGkMgEAor3qQvguVhl0
e5AsPgqRXBFgIiZaZUP5vLbL1m+K5wY=
=ZAi5
-----END PGP SIGNATURE-----

--MqPrBdX8kks83CUm--