Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

[Tom Zanussi <zanussi@kernel.org>]

On Sat, 2020-11-14 at 20:00 +0100, Mike Galbraith wrote:
> On Fri, 2020-11-13 at 14:14 -0600, Tom Zanussi wrote:
> > 
> > This patch seems to fix it for me:
> 
> If there was any discussion about this patch, I missed it.

There wasn't, just this thread.

> 
> > From 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e Mon Sep 17 00:00:00
> > 2001
> > Message-Id: <
> > 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e.1605297603.git.zanussi@kernel.org
> > >
> > From: Tom Zanussi <zanussi@kernel.org>
> > Date: Fri, 13 Nov 2020 13:04:15 -0600
> > Subject: [PATCH] net: Add missing __raw_write_seqcount_end() and
> >  seqcount_release()
> > 
> > The patch ('net: Properly annotate the try-lock for the seqlock")
> > adds
> > __raw_write_seqcount_begin() in qdisc_run_begin() but omits the
> > corresponding __raw_write_seqcount_end() and seqcount_release() in
> > qdisc_run_end().
> > 
> > Add it unconditionally, since qdisc_run_end() is never called
> > unless
> > qdisc_run_begin() succeeds, and if it succeeds,
> > __raw_write_seqcount_begin() seqcount_acquire() will have been
> > called.
> > 
> > Signed-off-by: Tom Zanussi <zanussi@kernel.org>
> > ---
> >  include/net/sch_generic.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> > index 112d2dca8b08..c5ccce4f8f62 100644
> > --- a/include/net/sch_generic.h
> > +++ b/include/net/sch_generic.h
> > @@ -192,7 +192,11 @@ static inline bool qdisc_run_begin(struct
> > Qdisc *qdisc)
> >  static inline void qdisc_run_end(struct Qdisc *qdisc)
> >  {
> >  #ifdef CONFIG_PREEMPT_RT
> > +	seqcount_t *s = &qdisc->running.seqcount;
> > +
> >  	write_sequnlock(&qdisc->running);
> > +	__raw_write_seqcount_end(s);
> > +	seqcount_release(&s->dep_map, 1, _RET_IP_);
> >  #else
> >  	write_seqcount_end(&qdisc->running);
> >  #endif
> 
> __raw_write_seqcount_end() is an integral part of write_sequnlock(),
> but we do seem to be missing a seqcount_release() in 5.4-rt.
> 

Yep, you're right, it's just the missing seqcount_release() - I'll
resubmit with just that.

Thanks,

Tom

> 	-Mike
>