Re: [PATCH v4 08/24] seqlock: lockdep assert non-preemptibility on seqcount_t write

Guenter Roeck
2020-08-08T23:23:08Z

On Sat, Aug 08, 2020 at 04:21:22PM -0700, Guenter Roeck wrote:
> On Mon, Jul 20, 2020 at 05:55:14PM +0200, Ahmed S. Darwish wrote:
> > Preemption must be disabled before entering a sequence count write side
> > critical section. Failing to do so, the seqcount read side can preempt
> > the write side section and spin for the entire scheduler tick. If that
> > reader belongs to a real-time scheduling class, it can spin forever and
> > the kernel will livelock.
> >
> > Assert through lockdep that preemption is disabled for seqcount writers.
> >
>
> This patch is causing compile failures for various images (eg arm:allmodconfig,
> arm:imx_v6_v7_defconfig, mips:allmodconfig).
>
> In file included from arch/arm/include/asm/bug.h:60,
> from include/linux/bug.h:5,
> from include/linux/thread_info.h:12,
> from include/asm-generic/current.h:5,
> from ./arch/arm/include/generated/asm/current.h:1,
> from include/linux/sched.h:12,
> from arch/arm/kernel/asm-offsets.c:11:
> include/linux/seqlock.h: In function 'write_seqcount_begin_nested':
> include/asm-generic/percpu.h:31:40: error: implicit declaration of function 'raw_smp_processor_id'
>

Also:

Building sparc64:allmodconfig ... failed
--------------
Error log:
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
In file included from arch/sparc/include/asm/bug.h:25,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/sparc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:15,
from include/linux/time.h:6,
from arch/sparc/vdso/vclock_gettime.c:16:
include/linux/seqlock.h: In function 'write_seqcount_begin_nested':
arch/sparc/include/asm/percpu_64.h:19:25: error: '__local_per_cpu_offset' undeclared

Again, reverting this patch fixes the problem.

Guenter

> Reverting it fixes the problem. Is this being addressed ?
>
> Guenter
>
> > Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> > ---
> > include/linux/seqlock.h | 29 +++++++++++++++++++++++------
> > 1 file changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
> > index e885702d8b82..54bc20496392 100644
> > --- a/include/linux/seqlock.h
> > +++ b/include/linux/seqlock.h
> > @@ -266,6 +266,12 @@ static inline void raw_write_seqcount_end(seqcount_t *s)
> > kcsan_nestable_atomic_end();
> > }
> >
> > +static inline void __write_seqcount_begin_nested(seqcount_t *s, int subclass)
> > +{
> > + raw_write_seqcount_begin(s);
> > + seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
> > +}
> > +
> > /**
> > * write_seqcount_begin_nested() - start a seqcount_t write section with
> > * custom lockdep nesting level
> > @@ -276,8 +282,19 @@ static inline void raw_write_seqcount_end(seqcount_t *s)
> > */
> > static inline void write_seqcount_begin_nested(seqcount_t *s, int subclass)
> > {
> > - raw_write_seqcount_begin(s);
> > - seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
> > + lockdep_assert_preemption_disabled();
> > + __write_seqcount_begin_nested(s, subclass);
> > +}
> > +
> > +/*
> > + * A write_seqcount_begin() variant w/o lockdep non-preemptibility checks.
> > + *
> > + * Use for internal seqlock.h code where it's known that preemption is
> > + * already disabled. For example, seqlock_t write side functions.
> > + */
> > +static inline void __write_seqcount_begin(seqcount_t *s)
> > +{
> > + __write_seqcount_begin_nested(s, 0);
> > }
> >
> > /**
> > @@ -575,7 +592,7 @@ static inline unsigned read_seqretry(const seqlock_t *sl, unsigned start)
> > static inline void write_seqlock(seqlock_t *sl)
> > {
> > spin_lock(&sl->lock);
> > - write_seqcount_begin(&sl->seqcount);
> > + __write_seqcount_begin(&sl->seqcount);
> > }
> >
> > /**
> > @@ -601,7 +618,7 @@ static inline void write_sequnlock(seqlock_t *sl)
> > static inline void write_seqlock_bh(seqlock_t *sl)
> > {
> > spin_lock_bh(&sl->lock);
> > - write_seqcount_begin(&sl->seqcount);
> > + __write_seqcount_begin(&sl->seqcount);
> > }
> >
> > /**
> > @@ -628,7 +645,7 @@ static inline void write_sequnlock_bh(seqlock_t *sl)
> > static inline void write_seqlock_irq(seqlock_t *sl)
> > {
> > spin_lock_irq(&sl->lock);
> > - write_seqcount_begin(&sl->seqcount);
> > + __write_seqcount_begin(&sl->seqcount);
> > }
> >
> > /**
> > @@ -649,7 +666,7 @@ static inline unsigned long __write_seqlock_irqsave(seqlock_t *sl)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&sl->lock, flags);
> > - write_seqcount_begin(&sl->seqcount);
> > + __write_seqcount_begin(&sl->seqcount);
> > return flags;
> > }
> >
> > --
> > 2.20.1
> >