From nobody Sat Feb 7 22:22:54 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73391CDB483 for ; Fri, 13 Oct 2023 08:46:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230192AbjJMIqe (ORCPT ); Fri, 13 Oct 2023 04:46:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48570 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230122AbjJMIqb (ORCPT ); Fri, 13 Oct 2023 04:46:31 -0400 Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C14BBC2 for ; Fri, 13 Oct 2023 01:46:29 -0700 (PDT) Received: by mail-ed1-x531.google.com with SMTP id 4fb4d7f45d1cf-53da80ada57so3301141a12.0 for ; Fri, 13 Oct 2023 01:46:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697186788; x=1697791588; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=+gnls/hmbBn1G4jzn/Z9Pf0jJBkrXUEs1i6LxoSc9Aw=; b=glzjXzNHahP5UtoAhMkLv4EA/6kFcy9IeeyqJL41Q9hNepB5Q53sGSR6vPvaGVtsQY AfJ3YMBDi0XcIoaFAsgHgphX/YDxm0T/QAqnTrIrzJ72LfQSzdDRKMBe+7QVcu0tcCtg Jnm8mVw/HZCC4EZNkosa7eKT0RysD3PqCNS7a6/uC/yIhDBEMddVSCX3VWDTgXOh7fGy jw7B4XIZPgtentUm5D/RtcnMWpdiUK5u/QfFijuwO79xRxJQHY8aFbOdgetc7T4RC+PG RcXvmMXJ4Z263UT7M2lxoSAUGR3xaxoYY8lAHfGU8LGWhVvn9DJWteuqonxs5g17X7ag 7pyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697186788; x=1697791588; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+gnls/hmbBn1G4jzn/Z9Pf0jJBkrXUEs1i6LxoSc9Aw=; b=K8IWiY6sV3OWlIgaT27FJ/ebQXHeRllEBwgjaPGffymPmgprImMrRzKNfvXIimiuEL V2KHKRM0eVQjKRTQH5F0JmEhflztzx+KdeFAzCT+MgBBWp1iMghJHBzeRfn4vWuwU7uj PrsYdx30cEaW7XtpExKx6evnHxPsRZf4MtD6rgDYuwRr2OB/cQ2xnhRxKEYAHMKJV1zs 83HyEY9HP3A7MmkA5YkbSTG9PF+ABmv+NTZZbBhbJ6k1p+3cXMVd8TNwt64suCGrIgIP s8s7quk87vjUbfB6mHrVkFsL91bb90SJwQj27jKD/tP3o3eFtWDpOTJbE2ApB/9iiM/H AV5A== X-Gm-Message-State: AOJu0Yx7Fom78jd1Gc6LdqttWfIBlKtWMdIx/2/zxiDtYmWgbsec9dpn rIO2g08nCKDJTWFaR02J2XBe8pujsX0= X-Google-Smtp-Source: AGHT+IHxTerL5vD5L9/paXKGDPVQsbH28UIZC6AXUGwTd61LPH1LBD73LbWCea2XN8BGKlbr5vg5tA== X-Received: by 2002:aa7:c393:0:b0:530:74ed:fc85 with SMTP id k19-20020aa7c393000000b0053074edfc85mr22142707edq.3.1697186787991; Fri, 13 Oct 2023 01:46:27 -0700 (PDT) Received: from gmail.com (1F2EF405.nat.pool.telekom.hu. [31.46.244.5]) by smtp.gmail.com with ESMTPSA id w15-20020aa7cb4f000000b0052a063e52b8sm11138997edt.83.2023.10.13.01.46.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Oct 2023 01:46:27 -0700 (PDT) Sender: Ingo Molnar Date: Fri, 13 Oct 2023 10:46:25 +0200 From: Ingo Molnar To: Linus Torvalds Cc: Oleg Nesterov , Peter Zijlstra , Alexey Gladkov , Ingo Molnar , Will Deacon , Waiman Long , Boqun Feng , linux-kernel@vger.kernel.org, Thomas Gleixner Subject: [PATCH] locking/seqlock: Propagate 'const' pointers within read-only methods, remove forced type casts Message-ID: References: <20231012143158.GA16133@redhat.com> <20231012143227.GA16143@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" * Linus Torvalds wrote: > On Thu, 12 Oct 2023 at 11:21, Ingo Molnar wrote: > > > > Okay, so dropping 'const' makes sense in terms of staying bug-compatible > > with the previous API and not build-breaking the world - but could we > > perhaps follow this up with fixups of the type misuse and then a removal > > of the forced type casts from these APIs? >=20 > No. The use of 'const' here is *not* a bug. >=20 > The thing is, 'const' doesn't mean what you seem to think it means. A > 'const' pointer in C in no way means that the target is constant - it > means that *THIS* use of the pointer will not write to the target! Yeah, that is absolutely what I too think 'const' means - and sorry, I=20 didn't expand: I meant something like the patch below, which clearly=20 separates the 'const' from the non-const pointer uses within=20 and removes the two forced type casts I was unhappy=20 about. The 'bug' was that the __seqprop_*ptr() wrapper was used with both const=20 and non-const pointers, and we forced a type and lost a tiny bit of=20 potential const propagation. The code was fine and I should not have called=20 it a 'bug', but I consider the dropping of 'const' a bad pattern, and I=20 sometimes exaggerate problems to trick^W convince developers to continue=20 working along a given path... In hindsight my "break the world" expectation was overblown too: our const=20 propagation through these methods was almost complete already, and the=20 fixes stayed within . This patch could probably be split into two patches. Lightly tested only. Does this work for you? Thanks, Ingo =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D> From: Ingo Molnar Date: Fri, 13 Oct 2023 10:15:46 +0200 Subject: [PATCH] locking/seqlock: Propagate 'const' pointers within read-on= ly methods, remove forced type casts Currently __seqprop_ptr() is an inline function that must chose to either use 'const' or non-const seqcount related pointers - but this results in the undesirable loss of 'const' propagation, via a forced type cast. The easiest solution would be to turn the pointer wrappers into macros that=20 pass through whatever type is passed to them - but the clever maze of=20 seqlock API instantiation macros relies on the GCC CPP '##' macro=20 extension, which isn't recursive, so inline functions must be used here. So create two wrapper variants instead: 'ptr' and 'const_ptr', and pick the right one for the codepaths that are const: read_seqcount_begin() and read_seqcount_retry(). This cleans up type handling and allows the removal of all type forcing. No change in functionality. Signed-off-by: Ingo Molnar Cc: Oleg Nesterov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Waiman Long Cc: Will Deacon Cc: Thomas Gleixner Cc: Paul E. McKenney Reviewed-by: Oleg Nesterov --- include/linux/seqlock.h | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h index 4b8dcd3a0d93..80f21d2ca2aa 100644 --- a/include/linux/seqlock.h +++ b/include/linux/seqlock.h @@ -200,9 +200,15 @@ typedef struct seqcount_##lockname { \ } seqcount_##lockname##_t; \ \ static __always_inline seqcount_t * \ -__seqprop_##lockname##_ptr(const seqcount_##lockname##_t *s) \ +__seqprop_##lockname##_ptr(seqcount_##lockname##_t *s) \ { \ - return (void *)&s->seqcount; /* drop const */ \ + return &s->seqcount; \ +} \ + \ +static __always_inline const seqcount_t * \ +__seqprop_##lockname##_const_ptr(const seqcount_##lockname##_t *s) \ +{ \ + return &s->seqcount; \ } \ \ static __always_inline unsigned \ @@ -247,9 +253,14 @@ __seqprop_##lockname##_assert(const seqcount_##locknam= e##_t *s) \ * __seqprop() for seqcount_t */ =20 -static inline seqcount_t *__seqprop_ptr(const seqcount_t *s) +static inline seqcount_t *__seqprop_ptr(seqcount_t *s) { - return (void *)s; /* drop const */ + return s; +} + +static inline const seqcount_t *__seqprop_const_ptr(const seqcount_t *s) +{ + return s; } =20 static inline unsigned __seqprop_sequence(const seqcount_t *s) @@ -302,6 +313,7 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, = mutex) __seqprop_case((s), mutex, prop)) =20 #define seqprop_ptr(s) __seqprop(s, ptr)(s) +#define seqprop_const_ptr(s) __seqprop(s, const_ptr)(s) #define seqprop_sequence(s) __seqprop(s, sequence)(s) #define seqprop_preemptible(s) __seqprop(s, preemptible)(s) #define seqprop_assert(s) __seqprop(s, assert)(s) @@ -353,7 +365,7 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, = mutex) */ #define read_seqcount_begin(s) \ ({ \ - seqcount_lockdep_reader_access(seqprop_ptr(s)); \ + seqcount_lockdep_reader_access(seqprop_const_ptr(s)); \ raw_read_seqcount_begin(s); \ }) =20 @@ -419,7 +431,7 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, = mutex) * Return: true if a read section retry is required, else false */ #define __read_seqcount_retry(s, start) \ - do___read_seqcount_retry(seqprop_ptr(s), start) + do___read_seqcount_retry(seqprop_const_ptr(s), start) =20 static inline int do___read_seqcount_retry(const seqcount_t *s, unsigned s= tart) { @@ -439,7 +451,7 @@ static inline int do___read_seqcount_retry(const seqcou= nt_t *s, unsigned start) * Return: true if a read section retry is required, else false */ #define read_seqcount_retry(s, start) \ - do_read_seqcount_retry(seqprop_ptr(s), start) + do_read_seqcount_retry(seqprop_const_ptr(s), start) =20 static inline int do_read_seqcount_retry(const seqcount_t *s, unsigned sta= rt) {