linux-kernel - Re: [PATCH 1/1] documentation: seqlock: fix the wrong documentation of read_seqbegin_or_lock/need

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251001190625.GA32506@redhat.com>
Date: Wed, 1 Oct 2025 21:06:26 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Waiman Long <llong@...hat.com>
Cc: Boqun Feng <boqun.feng@...il.com>, David Howells <dhowells@...hat.com>,
	Ingo Molnar <mingo@...hat.com>, Li RongQing <lirongqing@...du.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] documentation: seqlock: fix the wrong documentation
 of read_seqbegin_or_lock/need_seqretry

On 10/01, Waiman Long wrote:
>
> On 9/28/25 12:20 PM, Oleg Nesterov wrote:
> >
> >--- a/Documentation/locking/seqlock.rst
> >+++ b/Documentation/locking/seqlock.rst
> >@@ -218,13 +218,14 @@ Read path, three categories:
> >     according to a passed marker. This is used to avoid lockless readers
> >     starvation (too much retry loops) in case of a sharp spike in write
> >     activity. First, a lockless read is tried (even marker passed). If
> >-   that trial fails (odd sequence counter is returned, which is used as
> >-   the next iteration marker), the lockless read is transformed to a
> >-   full locking read and no retry loop is necessary::
> >+   that trial fails (sequence counter doesn't match), make the marker
> >+   odd for the next iteration, the lockless read is transformed to a
> >+   full locking read and no retry loop is necessary, for example::
> >  	/* marker; even initialization */
> >-	int seq = 0;
> >+	int seq = 1;
> >  	do {
> >+		seq++; /* 2 on the 1st/lockless path, otherwise odd */
> >  		read_seqbegin_or_lock(&foo_seqlock, &seq);
> >  		/* ... [[read-side critical section]] ... */
>
> It is kind of odd to initialize the sequence to 1 and add an sequence
> increment inside the loop.

Sure. But a) in this patch my only point is that the current documentation is
wrong, and b) the pseudo-code after this change becomes correct and the new
pattern already have the users. For example, do_io_accounting() and more.

> Perhaps we can do something like:

Perhaps. But could you please read the "RFC 2/1" thread? To me it is kind of
odd that the simple loops like this example have to even touch the sequence
counter inside the loop.

> +static inline int need_seqretry_once(seqlock_t *lock, int *seq)
> +{
> +       int ret = !(*seq & 1) && read_seqretry(lock, *seq);
> +
> +       if (ret)
> +               *seq = 1;       /* Enforce locking in next iteration */
> +       return ret;
> +}

And this is exactly what I tried to propose in "RFC 2/1". Plus more...

Oleg.