Re: [PATCH] kgdb: Avoid suspicious RCU usage warning

[Doug Anderson]

Hi,

On Wed, Jun 3, 2020 at 5:00 AM Daniel Thompson
<daniel.thompson@xxxxxxxxxx> wrote:
>
> On Tue, Jun 02, 2020 at 03:56:33PM -0700, Doug Anderson wrote:
> > > > 2. Perhaps remove the whole irq saving / restoring in kgdb_cpu_enter().
> > >
> > > Are you feeling lucky?
> > >
> > > I think there will come a time when bravery is called for but I'd rather
> > > see this as part of a bigger rewrite instead of a single high risk
> > > change.
> >
> > Hrm, maybe.  I guess it depends on whether we want to take baby steps
> > there or try to do it all at once.  If we take baby steps we will
> > occasionally fall down but we'll slowly start getting things cleaned
> > up.  If we wait for a full rewrite then we might be waiting for a long
> > time.  It'll also be harder to figure out which of the big changes in
> > the major rewrite broken someone.  ...or if the major rewrite comes in
> > 20 small/bisectable patches it may be hard to revert patch 2 out of 20
> > if the future patches all build upon it.  If we do one small high-risk
> > change and then wait before building upon it then it'll be easy for
> > someone to bisect and then yell for a revert.
>
> My views are a bit too nuanced for me to agree or disagree with this.
> I'm not against baby steps and I definitely *don't* want kgdb to
> continue to be preserved in aspic.
>
> However I'm still reluctant to start our baby steps with a "let's see
> if this breaks something" patch given we know it could be a very large
> number of kernel cycles before we get an answer.

Yeah, it's kinda amazing how much of a delay there is sometimes.
Perhaps all of the kgdb users are off on downstream kernels so they
only notice changes when they re-sync up.


> I would be much
> happier if those baby steps started, for example, with refactoring to
> decompose the beast into clearer (and dare I say better documented)
> functions.

OK, makes sense.


> Or put another way, even if someone sent me 20 small bisectable patches
> in a single kernel cycle I'd still want the high risk bits to be
> towards the end of the patch set.

OK, fair enough.  You're the maintainer so it's your view on the
matter that's the most important.  I guess one worry I have is that if
neither you nor I really understand the code path that a theoretical
system would take if it didn't already have its interrupts disabled
it'll be hard to keep from breaking it in making other changes to kgdb
in the future.

Anyway, enough discussion for now.  ;-)  My v2 should work like you
suggested and I'm not planning on any other short term changes to this
function, so we should be all good right now.

-Doug