On Fri 04-08-23 09:06:07, Doug Anderson wrote: > Hi, > > On Fri, Aug 4, 2023 at 8:02 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > It would have been slightly safer to modify arch_trigger_cpumask_backtrace > > > > by switching arguments so that some leftovers are captured easier. > > > > > > I'm not sure I understand. Oh, you're saying make the prototype of > > > arch_trigger_cpumask_backtrace() incompatible so that if someone is > > > directly calling it then it'll be a compile-time error? > > > > exactly. bool to int promotion would be too easy to miss while the > > pointer to int would complain loudly. > > > > > I guess the > > > hope is that nobody is calling that directly and they're calling > > > through the trigger_...() functions. > > > > Hope is one thing, being preventive another. > > > > > For now I'm going to leave this alone. > > > > If you are going to send another version then please consider this. Not > > a hard requirement but better. > > If I do send another version, do you have any suggestions for how to > change this to make it incompatible? I would swap parameters as this seems simplest. > I guess swapping the order of the > parameters would be best? I considered doing that for v4 but I felt > like long term the current order of the parameters was better. Yes the current ordering is better but having it other way around is not really horrendous either. > I also > considered a rename, but that different problems. ;-) If I rename both > the #define and the function then if someone has an out-of-tree patch > adding arch_trigger_cpumask_backtrace() for another architecture, like > say arm64, then there would be no compile-time failure indicating that > the out-of-tree patch needs updating. I could rename the functions but > _not_ the #define, I guess? I think that swapping would be simplest as the type mismatch should catch also pending out-of-tree potential implementations. -- Michal Hocko SUSE Labs
