[PATCH v4 05/17] watchdog/hardlockup: Rename touch_nmi_watchdog() to touch_hardlockup_watchdog()

[Doug Anderson]

Hi,

On Sun, May 7, 2023 at 6:35 PM Nicholas Piggin <npiggin at gmail.com> wrote:
>
> On Sat May 6, 2023 at 2:37 AM AEST, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, May 4, 2023 at 7:51 PM Nicholas Piggin <npiggin at gmail.com> wrote:
> > >
> > > On Fri May 5, 2023 at 8:13 AM AEST, Douglas Anderson wrote:
> > > > In preparation for the buddy hardlockup detector, rename
> > > > touch_nmi_watchdog() to touch_hardlockup_watchdog() to make it clear
> > > > that it will touch whatever hardlockup detector is configured. We'll
> > > > add a #define for the old name (touch_nmi_watchdog) so that we don't
> > > > have to touch every piece of code referring to the old name.
> > >
> > > Is this really helpful? Now it's got two names Could just leave it.
> > > If you insist then it'd be better just to rename everything in one
> > > go at the end of a merge window IMO. Conflicts would be trivial.
> >
> > I'm not picky here. I changed the name since Petr requested names to
> > be changed for any code I was touching [1] and so I threw this out as
> > a proposal. I agree that having two names can be confusing, but in
> > this case it didn't feel too terrible to me.
> >
> > I'd love to hear Petr's opinion on this name change. I'm happy with:
> >
> > a) This patch as it is.
> >
> > b) Dropping this patch (or perhaps just changing it to add comments).
> >
> > c) Changing this patch to rename all 70 uses of the old name. Assuming
> > this will go through Andrew Morton's tree, I'd be interested in
> > whether he's OK w/ this.
> >
> > d) Dropping this patch from this series but putting it on the
> > backburner to try to do later (so that the rename can happen at a time
> > when it's least disruptive).
> >
> >
> > > > Ideally this change would also rename the arch_touch_nmi_watchdog(),
> > > > but that is harder since arch_touch_nmi_watchdog() is exported with
> > > > EXPORT_SYMBOL() and thus is ABI. Add a comment next to the call to
> > > > hopefully alleviate some of the confusion here.
> > >
> > > We don't keep ABI fixed upstream.
> >
> > I'm happy to be corrected, but my understanding was that kernel devs
> > made an effort not to mess with things exported via "EXPORT_SYMBOL",
> > but things exported via "EXPORT_SYMBOL_GPL" were fair game.
>
> I don't think that's the case. If anything people might be a bit more
> inclined to accommodate GPL exports for out of tree modules that use
> them.
>
> > I guess maybe my patch calling it "ABI" is a stronger statement than
> > that, though. Doing a little more research, nobody wants to say that
> > things exported with "EXPORT_SYMBOL" are ABI, they just want to say
> > that we make an effort to have them be more stable.
>
> We wouldn't break any symbol for no reason, but in this case there is a
> good reason. If the name change is important for clarity then we change
> it. And this is about the easiest change for an out of tree module to
> deal with, so it should be no big deal for them.

OK, fair enough. My current plan is to wait a few more days to see if
anyone else chimes in with opinions. If I don't hear anything, in my
next version I will rename _neither_ touch_nmi_watchdog() nor
arch_touch_nmi_watchdog(). I'll still add comments indicating that
these functions touch the "hardlockup" watchdog but I won't attempt
the rename just to keep the series simpler.

-Doug