From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58B4DCA9EAF for ; Thu, 24 Oct 2019 17:15:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0AB9620659 for ; Thu, 24 Oct 2019 17:15:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0AB9620659 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 976DB6B0003; Thu, 24 Oct 2019 13:15:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9269A6B0006; Thu, 24 Oct 2019 13:15:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 83C466B0007; Thu, 24 Oct 2019 13:15:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0223.hostedemail.com [216.40.44.223]) by kanga.kvack.org (Postfix) with ESMTP id 629056B0003 for ; Thu, 24 Oct 2019 13:15:44 -0400 (EDT) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 0F5A39402 for ; Thu, 24 Oct 2019 17:15:44 +0000 (UTC) X-FDA: 76079330208.14.beds91_31fdffb2b6a12 X-HE-Tag: beds91_31fdffb2b6a12 X-Filterd-Recvd-Size: 7320 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf30.hostedemail.com (Postfix) with ESMTP for ; Thu, 24 Oct 2019 17:15:43 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E39F1B57; Thu, 24 Oct 2019 05:28:08 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A58543F71A; Thu, 24 Oct 2019 05:28:04 -0700 (PDT) Date: Thu, 24 Oct 2019 13:28:02 +0100 From: Mark Rutland To: Marco Elver Cc: akiyks@gmail.com, stern@rowland.harvard.edu, glider@google.com, parri.andrea@gmail.com, andreyknvl@google.com, luto@kernel.org, ard.biesheuvel@linaro.org, arnd@arndb.de, boqun.feng@gmail.com, bp@alien8.de, dja@axtens.net, dlustig@nvidia.com, dave.hansen@linux.intel.com, dhowells@redhat.com, dvyukov@google.com, hpa@zytor.com, mingo@redhat.com, j.alglave@ucl.ac.uk, joel@joelfernandes.org, corbet@lwn.net, jpoimboe@redhat.com, luc.maranget@inria.fr, npiggin@gmail.com, paulmck@linux.ibm.com, peterz@infradead.org, tglx@linutronix.de, will@kernel.org, kasan-dev@googlegroups.com, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-efi@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org Subject: Re: [PATCH v2 4/8] seqlock, kcsan: Add annotations for KCSAN Message-ID: <20191024122801.GD4300@lakrids.cambridge.arm.com> References: <20191017141305.146193-1-elver@google.com> <20191017141305.146193-5-elver@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191017141305.146193-5-elver@google.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Oct 17, 2019 at 04:13:01PM +0200, Marco Elver wrote: > Since seqlocks in the Linux kernel do not require the use of marked > atomic accesses in critical sections, we teach KCSAN to assume such > accesses are atomic. KCSAN currently also pretends that writes to > `sequence` are atomic, although currently plain writes are used (their > corresponding reads are READ_ONCE). > > Further, to avoid false positives in the absence of clear ending of a > seqlock reader critical section (only when using the raw interface), > KCSAN assumes a fixed number of accesses after start of a seqlock > critical section are atomic. Do we have many examples where there's not a clear end to a seqlock sequence? Or are there just a handful? If there aren't that many, I wonder if we can make it mandatory to have an explicit end, or to add some helper for those patterns so that we can reliably hook them. Thanks, Mark. > > Signed-off-by: Marco Elver > --- > include/linux/seqlock.h | 44 +++++++++++++++++++++++++++++++++++++---- > 1 file changed, 40 insertions(+), 4 deletions(-) > > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h > index bcf4cf26b8c8..1e425831a7ed 100644 > --- a/include/linux/seqlock.h > +++ b/include/linux/seqlock.h > @@ -37,8 +37,24 @@ > #include > #include > #include > +#include > #include > > +/* > + * The seqlock interface does not prescribe a precise sequence of read > + * begin/retry/end. For readers, typically there is a call to > + * read_seqcount_begin() and read_seqcount_retry(), however, there are more > + * esoteric cases which do not follow this pattern. > + * > + * As a consequence, we take the following best-effort approach for *raw* usage > + * of seqlocks under KCSAN: upon beginning a seq-reader critical section, > + * pessimistically mark then next KCSAN_SEQLOCK_REGION_MAX memory accesses as > + * atomics; if there is a matching read_seqcount_retry() call, no following > + * memory operations are considered atomic. Non-raw usage of seqlocks is not > + * affected. > + */ > +#define KCSAN_SEQLOCK_REGION_MAX 1000 > + > /* > * Version using sequence counter only. > * This can be used when code has its own mutex protecting the > @@ -115,6 +131,7 @@ static inline unsigned __read_seqcount_begin(const seqcount_t *s) > cpu_relax(); > goto repeat; > } > + kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); > return ret; > } > > @@ -131,6 +148,7 @@ static inline unsigned raw_read_seqcount(const seqcount_t *s) > { > unsigned ret = READ_ONCE(s->sequence); > smp_rmb(); > + kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); > return ret; > } > > @@ -183,6 +201,7 @@ static inline unsigned raw_seqcount_begin(const seqcount_t *s) > { > unsigned ret = READ_ONCE(s->sequence); > smp_rmb(); > + kcsan_atomic_next(KCSAN_SEQLOCK_REGION_MAX); > return ret & ~1; > } > > @@ -202,7 +221,8 @@ static inline unsigned raw_seqcount_begin(const seqcount_t *s) > */ > static inline int __read_seqcount_retry(const seqcount_t *s, unsigned start) > { > - return unlikely(s->sequence != start); > + kcsan_atomic_next(0); > + return unlikely(READ_ONCE(s->sequence) != start); > } > > /** > @@ -225,6 +245,7 @@ static inline int read_seqcount_retry(const seqcount_t *s, unsigned start) > > static inline void raw_write_seqcount_begin(seqcount_t *s) > { > + kcsan_begin_atomic(true); > s->sequence++; > smp_wmb(); > } > @@ -233,6 +254,7 @@ static inline void raw_write_seqcount_end(seqcount_t *s) > { > smp_wmb(); > s->sequence++; > + kcsan_end_atomic(true); > } > > /** > @@ -262,18 +284,20 @@ static inline void raw_write_seqcount_end(seqcount_t *s) > * > * void write(void) > * { > - * Y = true; > + * WRITE_ONCE(Y, true); > * > * raw_write_seqcount_barrier(seq); > * > - * X = false; > + * WRITE_ONCE(X, false); > * } > */ > static inline void raw_write_seqcount_barrier(seqcount_t *s) > { > + kcsan_begin_atomic(true); > s->sequence++; > smp_wmb(); > s->sequence++; > + kcsan_end_atomic(true); > } > > static inline int raw_read_seqcount_latch(seqcount_t *s) > @@ -398,7 +422,9 @@ static inline void write_seqcount_end(seqcount_t *s) > static inline void write_seqcount_invalidate(seqcount_t *s) > { > smp_wmb(); > + kcsan_begin_atomic(true); > s->sequence+=2; > + kcsan_end_atomic(true); > } > > typedef struct { > @@ -430,11 +456,21 @@ typedef struct { > */ > static inline unsigned read_seqbegin(const seqlock_t *sl) > { > - return read_seqcount_begin(&sl->seqcount); > + unsigned ret = read_seqcount_begin(&sl->seqcount); > + > + kcsan_atomic_next(0); /* non-raw usage, assume closing read_seqretry */ > + kcsan_begin_atomic(false); > + return ret; > } > > static inline unsigned read_seqretry(const seqlock_t *sl, unsigned start) > { > + /* > + * Assume not nested: read_seqretry may be called multiple times when > + * completing read critical section. > + */ > + kcsan_end_atomic(false); > + > return read_seqcount_retry(&sl->seqcount, start); > } > > -- > 2.23.0.866.gb869b98d4c-goog >