From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (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 EEDF235963 for ; Wed, 18 Dec 2024 03:30:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734492645; cv=none; b=D/sRGGa4l5fSuJq7P4zRBStfBxHpVJM57aHjoz+NMKPufQ4x617R4XehFUGBpQ3JpsJttBdDAdRAbZHvhlAa9Fj3VMbq5OSw/hZzQTY0xkjExlGHOnBNK0IKYpeX1IaSRL3A4VHRbwSyJbcs2mjRniTdLKAJKXIiXyHvbKW/BMQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734492645; c=relaxed/simple; bh=dVTxy5zGDjp/6yq6ulQ4MpIhJWc/yX0F+DwGq3J/Q1o=; h=From:Message-ID:Date:MIME-Version:Subject:To:Cc:References: In-Reply-To:Content-Type; b=P7p3/WeS49l7ybwvZrV48KDJXYEorZKu4zMCeMi/tsUVlpfMM/C4LuIgWaP6YN95ZksvhpT5nWrG1A4XdSEeARLHhEtRra5+MPvC0r4Ci9fRDQzVaidWGjJOr34iSN6DG4eiVmzAfpeDu+CQ5siYL1ZS3eqxWy9nNUfkpnVENNg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=WOmF8Hcg; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="WOmF8Hcg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734492642; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Yc2YBZdhSJnterQdNUvFxd8LVnSN7oUD3OAblB3ShS4=; b=WOmF8HcghhVBUdZIuyARi+8SrZS9KKS11x4e6zJDZIEN/Bzfvolb7EwMW1gHOrfeqqHzHT RbLYkqlZTB9v0rByBiorVo8CVVCK7VvK5yCwCPAVxKJyqi8T8Syygg/yB/8afCDQrY8q/n BsvkFjXDiCpmEr/Rb92oKM20FJS36lU= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-503-GYXZw5KKPGy1ooVr1lVNKw-1; Tue, 17 Dec 2024 22:30:41 -0500 X-MC-Unique: GYXZw5KKPGy1ooVr1lVNKw-1 X-Mimecast-MFC-AGG-ID: GYXZw5KKPGy1ooVr1lVNKw Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-6d8f14fc179so30679486d6.2 for ; Tue, 17 Dec 2024 19:30:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734492641; x=1735097441; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Yc2YBZdhSJnterQdNUvFxd8LVnSN7oUD3OAblB3ShS4=; b=Jvgz/20NLYE6cVBjv39EjOWiC+3QXt77SWXzD0Hbbk/eWhNihuvCexH91iZ6Kkiqsn 3dQ8LTZ8ziCYR4zTCzNgcf4Ek8YTTvLxbHEB/u4jigIObdRLlMgGAbkpxE/4/nL193pM uWtnzVLWu0YkLJ1goz7q7IDu9VN8mNryDi8fSWqq4QA86TSkz8Vb37KeDZTqaRuMNj7B 3KXLdJueY/I5Km3AEZJmURC0HlgvYNYzF/7qR85JaN5Fnfn3xbXCWVpnfDGKyAq21AdJ D9sz1hZSDEBnXrkHpxMB6nVaS32jd3ku29liy106prm8a+BnOdsrHivEFpEM/LzmTqEk HqQA== X-Forwarded-Encrypted: i=1; AJvYcCVxR8cJUY46MeH8yCVQquKb2jBxXb7Wd22rvzQRayKe5qCN08EZztIcMDvyXjyE5766AFNao1z5UBley1k=@vger.kernel.org X-Gm-Message-State: AOJu0YwEKRppkXKwBur4G9sVTqLz3wmxZIJ5JFnjbrqis/MZlaSieAoq UT7xBG8e2dEdQ9leImvFlPHkX1k/Bcip2tuBfntVvasc5rX5m+RO7f0slG0QJr+Y5ppc2H+1svL 2UKXO2ewxxGU2fUL6WpCCIBhcYj4B3LsCl9gn3YAu8RdBZ1OkD78JgXK5rOJgeA== X-Gm-Gg: ASbGncuDKhG8LmEBV+yaUHMOtYvZE9Ey+vXlbqP+1POZBtqBL2dBJS9ujEv2GsT3h9G MgqN6ZCQb83GlzEmDASADcR4HwWNF4k9OkjQBqvesO0Sb69Olh0cSPrKBxv7ollyAvQsaA2ATQb BXYznSR1t9vY0LWDjDcToVbTr3VzQJ944LM7ZAUqTeh5Mg/Ghz45Bz+MzpbpQSb7vZMuIZH/vPH iGuVsJwVUUnLT40LQzqfL2zokkLTkPv9DAsSLRCDWsFJYCPH+shdGnCCcMfObmFa0KHyqpOxhQM wKkMFrwDHOzKwWfCbFVHGcmL X-Received: by 2002:a05:6214:2586:b0:6d8:932a:eaa3 with SMTP id 6a1803df08f44-6dd091a0732mr28127006d6.3.1734492641159; Tue, 17 Dec 2024 19:30:41 -0800 (PST) X-Google-Smtp-Source: AGHT+IGKwPwdS9vH9tMM2iB2wAQXXEaKs91N7QtAPnk16pxK221Wiid1yseV9dNVHoIK4Sl6ODHzbQ== X-Received: by 2002:a05:6214:2586:b0:6d8:932a:eaa3 with SMTP id 6a1803df08f44-6dd091a0732mr28126676d6.3.1734492640834; Tue, 17 Dec 2024 19:30:40 -0800 (PST) Received: from ?IPV6:2601:188:ca00:a00:f844:fad5:7984:7bd7? ([2601:188:ca00:a00:f844:fad5:7984:7bd7]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6dccd25a991sm45627286d6.49.2024.12.17.19.30.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Dec 2024 19:30:40 -0800 (PST) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: <6fbd26a5-615f-41bf-ad54-91114898cc2c@redhat.com> Date: Tue, 17 Dec 2024 22:30:39 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] seqlock: Use WRITE_ONCE() when updating sequence To: Daniel Xu , mingo@redhat.com, will@kernel.org, peterz@infradead.org Cc: boqun.feng@gmail.com, linux-kernel@vger.kernel.org, paulmck@kernel.org References: <0eaea03ecc9df536649763cfecda356fc38b6938.1734477414.git.dxu@dxuuu.xyz> Content-Language: en-US In-Reply-To: <0eaea03ecc9df536649763cfecda356fc38b6938.1734477414.git.dxu@dxuuu.xyz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 12/17/24 6:17 PM, Daniel Xu wrote: > `sequence` is a concurrently accessed shared variable on the reader > side. Therefore, it needs to be wrapped in WRITE_ONCE() in order to > prevent unwanted compiler optimizations like store tearing. > > Signed-off-by: Daniel Xu > --- > include/linux/seqlock.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h > index 5298765d6ca4..f4c6f2507742 100644 > --- a/include/linux/seqlock.h > +++ b/include/linux/seqlock.h > @@ -45,7 +45,7 @@ static inline void __seqcount_init(seqcount_t *s, const char *name, > * Make sure we are not reinitializing a held lock: > */ > lockdep_init_map(&s->dep_map, name, key, 0); > - s->sequence = 0; > + WRITE_ONCE(s->sequence, 0); > } > The init function certainly doesn't need to use WRITE_ONCE(). > #ifdef CONFIG_DEBUG_LOCK_ALLOC > @@ -405,7 +405,7 @@ do { \ > static inline void do_raw_write_seqcount_begin(seqcount_t *s) > { > kcsan_nestable_atomic_begin(); > - s->sequence++; > + WRITE_ONCE(s->sequence, READ_ONCE(s->sequence) + 1); > smp_wmb(); > } > > @@ -426,7 +426,7 @@ do { \ > static inline void do_raw_write_seqcount_end(seqcount_t *s) > { > smp_wmb(); > - s->sequence++; > + WRITE_ONCE(s->sequence, READ_ONCE(s->sequence) + 1); > kcsan_nestable_atomic_end(); > } > > @@ -548,9 +548,9 @@ static inline void do_write_seqcount_end(seqcount_t *s) > static inline void do_raw_write_seqcount_barrier(seqcount_t *s) > { > kcsan_nestable_atomic_begin(); > - s->sequence++; > + WRITE_ONCE(s->sequence, READ_ONCE(s->sequence) + 1); > smp_wmb(); > - s->sequence++; > + WRITE_ONCE(s->sequence, READ_ONCE(s->sequence) + 1); > kcsan_nestable_atomic_end(); > } > > @@ -569,7 +569,7 @@ static inline void do_write_seqcount_invalidate(seqcount_t *s) > { > smp_wmb(); > kcsan_nestable_atomic_begin(); > - s->sequence+=2; > + WRITE_ONCE(s->sequence, READ_ONCE(s->sequence) + 2); > kcsan_nestable_atomic_end(); > } > > @@ -673,7 +673,7 @@ read_seqcount_latch_retry(const seqcount_latch_t *s, unsigned start) > static __always_inline void raw_write_seqcount_latch(seqcount_latch_t *s) > { > smp_wmb(); /* prior stores before incrementing "sequence" */ > - s->seqcount.sequence++; > + WRITE_ONCE(s->seqcount.sequence, READ_ONCE(s->seqcount.sequence) + 1); > smp_wmb(); /* increment "sequence" before following stores */ > } > For seqcount, its actual value isn't important. What is important is whether the value changes and whether it is even or odd. So even if store tearing is happening, it shouldn't affect its operation. I doubt we need to use WRITE_ONCE() here. Could you come up with a scenario where store tearing will make it behave incorrectly? Cheers, Longman