From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 481C91853; Mon, 24 Feb 2025 11:15:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740395735; cv=none; b=duBT3LtD9mwn4zmE8tf8ag8zHkShaX6rowJ6zegMQheS4JMnM6LEEDDDXwtiFMwfSgU/aLi82POlYCs3OcGGz9wHatZPx1lwuObdBi77RTuo5QNVdHRooFXdrYxCrtoh0AEWq6CgDwZ+89FfUbfTmrP+KjqU8/bMqcbRsKTWF6s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740395735; c=relaxed/simple; bh=WmHoHIDzRoE4YEADCig2y1QWlSoJdSm+eIGybIJt26k=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=Os6Sn+r5M2ZFW4OOp4VFy20UHWPrC5ZsJoKih31wJ2iC9nxOajrASYzv3A3gKyUueT/VgQcPbWgaVjTP4C+6TiouNb8YwzjoI8phVwP4IEYOh4GclWKkWL/k1vt/uQkl4z86ojy5jurXQXrcOIe+j3CbM01Zjt1vYZC2+irfUFM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=CvlTaxKA; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="CvlTaxKA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B13AFC4CED6; Mon, 24 Feb 2025 11:15:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740395733; bh=WmHoHIDzRoE4YEADCig2y1QWlSoJdSm+eIGybIJt26k=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=CvlTaxKAcOGRpLRF3L/NiwDx7cBxWbrtcO112/NiNe4V5wi6sXfTpZ79dUqghuFnJ aiK0X20dkEwTgw9ABq+ibZ4QH8ZBCoTSb9DmwQa+ORE2JOvT5Rtio6J3/5Dr7YwPNt AmpWPokx6sV3fSEJRuLn3gnMFdkJPDEYaAcZWNfEvjX+WjPyZWNWe7uPFDuRXk3UaO RlRxDoR0F8/MFuqhg6ExsAfWCXl3J+9NcVK8l42dpqbaoTEDWzUF/NrmzG/YtqGk2X I61b2YmiktKl8pp+OsNTlGG5344kTx11dT6GZgtWqO3h3NU0UcBFOGwCteDC1Kv/uD zjDEyGIXKUe7A== From: Andreas Hindborg To: "Benno Lossin" Cc: "Boqun Feng" , , , "Peter Zijlstra" , "Ingo Molnar" , "Will Deacon" , "Waiman Long" , "Miguel Ojeda" , "Alex Gaynor" , "Gary Guo" , =?utf-8?Q?Bj=C3=B6rn?= Roy Baron , "Alice Ryhl" , "Trevor Gross" Subject: Re: [PATCH] rust: sync: lock: Add an example for Guard::lock_ref() In-Reply-To: <304505cb-9f68-4d34-b4f1-7d703baba012@proton.me> (Benno Lossin's message of "Mon, 24 Feb 2025 10:06:35 +0000") References: <20250223072114.3715-1-boqun.feng@gmail.com> <87wmdf22ae.fsf@kernel.org> <304505cb-9f68-4d34-b4f1-7d703baba012@proton.me> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Mon, 24 Feb 2025 12:15:22 +0100 Message-ID: <87r03n1tmd.fsf@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain "Benno Lossin" writes: > On 24.02.25 09:08, Andreas Hindborg wrote: >> Boqun Feng writes: >> >>> To provide examples on usage of `Guard::lock_ref()` along with the unit >>> test, an "assert a lock is held by a guard" example is added. >>> >>> Signed-off-by: Boqun Feng >>> --- >>> This depends on Alice's patch: >>> >>> https://lore.kernel.org/all/20250130-guard-get-lock-v1-1-8ed87899920a@google.com/ >>> >>> I'm also OK to fold this in if Alice thinks it's fine. >>> >>> rust/kernel/sync/lock.rs | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs >>> index 3701fac6ebf6..6d868e35b0a3 100644 >>> --- a/rust/kernel/sync/lock.rs >>> +++ b/rust/kernel/sync/lock.rs >>> @@ -201,6 +201,30 @@ unsafe impl Sync for Guard<'_, T, B> {} >>> >>> impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> { >>> /// Returns the lock that this guard originates from. >>> + /// >>> + /// # Examples >>> + /// >>> + /// The following example shows how to use [`Guard::lock_ref()`] to assert the corresponding >>> + /// lock is held. >>> + /// >>> + /// ``` >>> + /// # use kernel::{new_spinlock, stack_pin_init, sync::lock::{Backend, Guard, Lock}}; >>> + /// >>> + /// fn assert_held(guard: &Guard<'_, T, B>, lock: &Lock) { >>> + /// // Address-equal means the same lock. >>> + /// assert!(core::ptr::eq(guard.lock_ref(), lock)); >>> + /// } >> >> This seems super useful. Perhaps add this method as part of the lock api >> instead of just having it in the example? > > I don't think it should be an assert. Instead make it return a > `Result<(), ()>`. (or create better named unit error types) No, this should not be part of usual control flow, and developers should not make control flow decisions based on this. It would always be an assertion. But you are right that `assert!` is probably not what we want. `debug_assert!` might be fine though. Best regards, Andreas Hindborg