Received: by 2002:a25:e74b:0:0:0:0:0 with SMTP id e72csp345517ybh; Mon, 20 Jul 2020 18:47:01 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy2+1rFDGd5e2HlstH/DAew/RJDjDCgSyFk8RIUsmjzz1VObkNVvr/O1gv/HZIbGKqtgTdz X-Received: by 2002:a17:906:700f:: with SMTP id n15mr24442293ejj.390.1595296021165; Mon, 20 Jul 2020 18:47:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1595296021; cv=none; d=google.com; s=arc-20160816; b=QHKiT+MQMj9DL4DSSdXdGdjsD5/uwgLBFZBI0x6CeTzxqFqcvWofww4A7wkNcfL7HS 95Tt/PRPzw4JiMtssen/airjk+f5Y7ckPdN5aFqjvY+ABf8JTFt0T49H6dXD9Cm9Gc4e U2DPQJWwfvl5x5l+qr24oqcnpY/M7G9KjruftwBhDctgAVTF8LRYmRlz7Xt2zYZT5bYE aeRyCybX6yMzpAVYOWpTwi1V7ow6hjbCh2lKqhQ5tULU6izXf9Elbz1fz9GWPUC7mNID Z3yeNiYCNvy1kt4GPYK/XZyvGRdqwjT3YqCoiGpt35WxIF1/qdhVpWfgb1AEcDWQNoEM 9/UQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date; bh=+KR4qFj8IVlDkuGrA/GzxsfQe714KbygUnT/vZxQn3E=; b=YTisdZUvr/Wc+RW8sDkePEnL+Ewp9oGG6habkXK8gEEpprJLXM14zwwg33JNQFPeBU zaO5bZQbv22+OsTHsXngSmtPmozX0wIGHOmiVMgE46wHssYIqNA1uqCjhy78lIaUhIrS FGnbkVgZKABaRqygZQNbStpWkRlBm2oYi7UGoKoDCJRikhzc8sz9lEq2rgPlXE+KWxxW nGYZDTRnV1ymB1x5LMwBwIIMisbw/YJtaJehLvO5wMFQqPf/DkqEjkUYJ6Ib4RgsETNW RQ5tEDraDAK2FHHNO+XFcWJQqG0uNo/yj06apfbuzitrPSvYBXrFE6t3deZ9y5RkZMpY C9VQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id co19si11297293edb.605.2020.07.20.18.46.37; Mon, 20 Jul 2020 18:47:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726892AbgGUBoE (ORCPT + 99 others); Mon, 20 Jul 2020 21:44:04 -0400 Received: from mail.kernel.org ([198.145.29.99]:45550 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725954AbgGUBoD (ORCPT ); Mon, 20 Jul 2020 21:44:03 -0400 Received: from oasis.local.home (cpe-66-24-58-225.stny.res.rr.com [66.24.58.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE3042075B; Tue, 21 Jul 2020 01:44:01 +0000 (UTC) Date: Mon, 20 Jul 2020 21:44:00 -0400 From: Steven Rostedt To: "Ahmed S. Darwish" Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , Thomas Gleixner , "Paul E. McKenney" , "Sebastian A. Siewior" , LKML , Jonathan Corbet , linux-doc@vger.kernel.org Subject: Re: [PATCH v4 01/24] Documentation: locking: Describe seqlock design and usage Message-ID: <20200720214400.5ec6a028@oasis.local.home> In-Reply-To: <20200720155530.1173732-2-a.darwish@linutronix.de> References: <20200519214547.352050-1-a.darwish@linutronix.de> <20200720155530.1173732-1-a.darwish@linutronix.de> <20200720155530.1173732-2-a.darwish@linutronix.de> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 20 Jul 2020 17:55:07 +0200 "Ahmed S. Darwish" wrote: > +++ b/include/linux/seqlock.h > @@ -1,36 +1,15 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > #ifndef __LINUX_SEQLOCK_H > #define __LINUX_SEQLOCK_H > + > /* > - * Reader/writer consistent mechanism without starving writers. This type of > - * lock for data where the reader wants a consistent set of information > - * and is willing to retry if the information changes. There are two types > - * of readers: > - * 1. Sequence readers which never block a writer but they may have to retry > - * if a writer is in progress by detecting change in sequence number. > - * Writers do not wait for a sequence reader. > - * 2. Locking readers which will wait if a writer or another locking reader > - * is in progress. A locking reader in progress will also block a writer > - * from going forward. Unlike the regular rwlock, the read lock here is > - * exclusive so that only one locking reader can get it. > + * seqcount_t / seqlock_t - a reader-writer consistency mechanism with > + * lockless readers (read-only retry loops), and no writer starvation. > * > - * This is not as cache friendly as brlock. Also, this may not work well > - * for data that contains pointers, because any writer could > - * invalidate a pointer that a reader was following. > + * See Documentation/locking/seqlock.rst I absolutely hate it when I see this. I much rather have the documentation next to the code. Because honestly, I trust that comments next to the code will get updated if the code changes much more likely than comments buried in the Documentation directory. It's also more likely that I wont even bother looking at the doc (because I wont trust it to be up to date) and just read the code and try to figure it out. Or look at how others have used it. -- Steve > * > - * Expected non-blocking reader usage: > - * do { > - * seq = read_seqbegin(&foo); > - * ... > - * } while (read_seqretry(&foo, seq)); > - * > - * > - * On non-SMP the spin locks disappear but the writer still needs > - * to increment the sequence variables because an interrupt routine could > - * change the state of the data. > - * > - * Based on x86_64 vsyscall gettimeofday > - * by Keith Owens and Andrea Arcangeli > + * Copyrights: > + * - Based on x86_64 vsyscall gettimeofday: Keith Owens, Andrea Arcangeli > */ > > #include > @@ -41,8 +20,8 @@ > #include > > /* > - * The seqlock interface does not prescribe a precise sequence of read > - * begin/retry/end. For readers, typically there is a call to > + * The seqlock seqcount_t 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. > * > @@ -50,16 +29,30 @@ > * via seqcount_t under KCSAN: upon beginning a seq-reader critical section, > * pessimistically mark the 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. Usage of seqlocks via seqlock_t > - * interface is not affected. > + * memory operations are considered atomic. Usage of the seqlock_t interface > + * 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 > - * updating starting before the write_seqcountbeqin() and ending > - * after the write_seqcount_end(). > + * Sequence counters (seqcount_t) > + * > + * This is the raw counting mechanism, without any writer protection. > + * > + * Write side critical sections must be serialized and non-preemptible. > + * > + * If readers can be invoked from hardirq or softirq contexts, > + * interrupts or bottom halves must also be respectively disabled before > + * entering the write section. > + * > + * This mechanism can't be used if the protected data contains pointers, > + * as the writer can invalidate a pointer that a reader is following. > + * > + * If it's desired to automatically handle the sequence counter writer > + * serialization and non-preemptibility requirements, use a sequential > + * lock (seqlock_t) instead. > + * > + * See Documentation/locking/seqlock.rst > */ > typedef struct seqcount { > unsigned sequence; > @@ -398,10 +391,6 @@ static inline void raw_write_seqcount_latch(seqcount_t *s) > smp_wmb(); /* increment "sequence" before following stores */ > } > > -/* > - * Sequence counter only version assumes that callers are using their > - * own mutexing. > - */ > static inline void write_seqcount_begin_nested(seqcount_t *s, int subclass) > { > raw_write_seqcount_begin(s); > @@ -434,15 +423,21 @@ static inline void write_seqcount_invalidate(seqcount_t *s) > kcsan_nestable_atomic_end(); > } > > +/* > + * Sequential locks (seqlock_t) > + * > + * Sequence counters with an embedded spinlock for writer serialization > + * and non-preemptibility. > + * > + * For more info, see: > + * - Comments on top of seqcount_t > + * - Documentation/locking/seqlock.rst > + */ > typedef struct { > struct seqcount seqcount; > spinlock_t lock; > } seqlock_t; > > -/* > - * These macros triggered gcc-3.x compile-time problems. We think these are > - * OK now. Be cautious. > - */ > #define __SEQLOCK_UNLOCKED(lockname) \ > { \ > .seqcount = SEQCNT_ZERO(lockname), \