linux-kernel - Re: [PATCH 1/4] seqlock: introduce scoped_seqlock_read() and scoped_seqlock_read

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20251007171810.GC12329@redhat.com>
Date: Tue, 7 Oct 2025 19:18:10 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Waiman Long <llong@...hat.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
	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/4] seqlock: introduce scoped_seqlock_read() and
 scoped_seqlock_read_irqsave()

On 10/07, Waiman Long wrote:
>
> On 10/7/25 10:21 AM, Oleg Nesterov wrote:
> >+
> >+/* internal helper for scoped_seqlock_read/scoped_seqlock_read_irqsave */
> >+static inline int
> >+scoped_seqlock_read_retry(seqlock_t *lock, int *seq, unsigned long *flags)
> I would suggest adding the "__" prefix to indicate that this is an internal
> helper that shouldn't be called directly.

OK, I will add "__", but I thought that "internal helper" makes it clear that
it shouldn't be called directly. Nevermind, will do.

> >+#define __scoped_seqlock_read(lock, lockless, seq)	\
> >+	for (int lockless = 1, seq = read_seqbegin(lock);		\
> >+	     lockless || scoped_seqlock_read_retry(lock, &seq, NULL);	\
> >+	     lockless = 0)
>
> I like Linus' suggestion of putting lockless and seq into a struct to make
> it more consistent with __scoped_seqlock_read_irqsave().

Again, will do. See my reply to Linus.

> >+/**
> >+ * scoped_seqlock_read_irqsave(lock) - same as scoped_seqlock_read() but
> >+ *                                     disables irqs on a locking pass
> >+ * @lock: pointer to the seqlock_t protecting the data
> Maybe we should we should add a comment saying that this API is similar to
> scoped_seqlock_read() but with irqs disabled.

Hmm... This is what the comment above tries to say... Do you think it can
be improved?

Oleg.