Re: [PATCH 1/4] seqlock: introduce scoped_seqlock_read() and scoped_seqlock_read_irqsave()

[Oleg Nesterov]

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.