Received: by 2002:a25:2c96:0:0:0:0:0 with SMTP id s144csp965173ybs; Mon, 25 May 2020 03:55:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxPK5ZkLPHgJPgvZO3sZ1Bj8Hre9UQr0xxxG1n/n3A+j9hKBk5JKCDZPqwwvYZJQuZ1oxEV X-Received: by 2002:a50:d7c7:: with SMTP id m7mr15023419edj.101.1590404130488; Mon, 25 May 2020 03:55:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1590404130; cv=none; d=google.com; s=arc-20160816; b=bOPG0mRp6EEYWTJreRA2xzbRdCRHGC7MHLiwvUbSakTFkUa5gobRMAyYIlM0k1tmyr CfbDZ1jzO31n1cdeRgLJ1fsNTxo77w2th7L+1LFIUmXTMeCEOIJLSI+mFAhnSE28yrLz R4EFZDMbTXHytjb7PWrbKdjwDQ2ZMRJgkIut7jMLGt3ugi33cXH4sPGZ3h+IYBuR+PND s4u3eAM76wEgkEgoOMG1PyRaQBSJEGjkiQuYKW6pcN5JoxXRP9j84ey2IjQwEcaQN0N+ 8PnVO54HwiIym6vo6QPk7NMWeUhfemWqhk4xYInIQ5ee9hHklbmgcffJm/rXT25jYXwo DRgQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=u7SbmGibf7dPTCGCPn08kUHMA3dAFUt5xq0XfwVJI/w=; b=ACtp04pdkwdSedX4Vl6XfghRcc21PSF1WF74qpeWESOq2e7TIE54TyT+EUp5SlyEa+ D8Xvi9Zs4Vd0ptKz7EQfsRKjmbd6vKk57smYR0ZMHBV8HTp0bw1kNWEfu2VNYEn7qm78 d/9w//ik4kYy1FSQ2+ZHxAZpBRzj+Tzc4kayhtz89EFGjiMYEt2PoQWv3lZIK1BM6S/F WP/TStD06smhXtxAk6QCvGJnVTPEXPM6HpmF059MR90JLh7PkdmLjJ0Cxv6GCT4AtiMT o1k/j9Lsw1Mv+sdFQEsT6ovJvp83fdIQNMl9A2CHVfWXrpbNJJa9EPxbv1X3plNxbsQt TEKQ== 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 f8si10048674edk.160.2020.05.25.03.55.06; Mon, 25 May 2020 03:55:30 -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 S2389924AbgEYKu2 (ORCPT + 99 others); Mon, 25 May 2020 06:50:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389484AbgEYKu1 (ORCPT ); Mon, 25 May 2020 06:50:27 -0400 Received: from Galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B3E56C061A0E; Mon, 25 May 2020 03:50:27 -0700 (PDT) Received: from [5.158.153.53] (helo=debian-buster-darwi.lab.linutronix.de) by Galois.linutronix.de with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1jdAgQ-0006wF-Fg; Mon, 25 May 2020 12:50:14 +0200 Date: Mon, 25 May 2020 12:50:12 +0200 From: "Ahmed S. Darwish" To: Steven Rostedt 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 v1 09/25] Documentation: locking: Describe seqlock design and usage Message-ID: <20200525105012.GA375313@debian-buster-darwi.lab.linutronix.de> References: <20200519214547.352050-1-a.darwish@linutronix.de> <20200519214547.352050-10-a.darwish@linutronix.de> <20200522180145.GR325280@hirez.programming.kicks-ass.net> <20200522182409.4016d83c@oasis.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200522182409.4016d83c@oasis.local.home> User-Agent: Mutt/1.10.1 (2018-07-13) X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Steven Rostedt wrote: > Peter Zijlstra wrote: > > On Tue, May 19, 2020 at 11:45:31PM +0200, Ahmed S. Darwish wrote: > > > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h > > > index d35be7709403..2a4af746b1da 100644 > > > --- a/include/linux/seqlock.h > > > +++ 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 for full description. > > > > So I really really hate that... I _much_ prefer code comments to crappy > > documents. > > Agreed. Comments are much less likely to bitrot than documents. The > farther away the documentation is from the code, the quicker it becomes > stale. > > It's fine to add "See Documentation/..." but please don't *ever* remove > comments that's next to the actual code. > This patch was unfairly cut at the hunk above :) If you follow the rest of it, you see that the documentation has just moved 3 lines below: /* - * 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) + * + * The raw counting mechanism without any writer protection. Write side + * critical sections must be serialized and readers on the same CPU + * (e.g. through preemption or interrupts) must be excluded. + * + * 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 { and: +/* + * Sequential locks (seqlock_t) + * + * Sequence counters with an embedded spinlock for writer serialization + * and non-preemptibility. + * + * See Documentation/locking/seqlock.rst + */ + typedef struct { struct seqcount seqcount; spinlock_t lock; } seqlock_t; This was done because, as said in the commit log, documentation of seqcount_t and seqlock_t was originally intermingled. This is incorrect and confusing since the usage constrains for each type are vastly different. Then, the brlock comment: 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. was removed not because it's moved to Documentation/locking/seqlock.rst, but because it's obsolete: 0f6ed63b1707 ("no need to keep brlock macros anymore..."). Thanks, -- Ahmed S. Darwish Linutronix GmbH