On Tue, Feb 06, 2018 at 02:06:50PM +0000, Mathieu Desnoyers wrote: > ----- On Feb 6, 2018, at 8:55 AM, Will Deacon will.deacon@xxxxxxx wrote: > > On Tue, Feb 06, 2018 at 12:52:34PM +0000, Mathieu Desnoyers wrote: > >> One approach I would consider for this is to duplicate this > >> comment and add it just above the "eret" instruction within the > >> macro: > >> > >> /* > >> * ARCH_HAS_MEMBARRIER_SYNC_CORE rely on eret context synchronization > >> * when returning from IPI handler, and when returning to user-space. > >> */ > >> > >> Or perhaps Will has something else in mind ? > > > > To be honest with you, I'd just drop the comment entirely. entry.S is > > terrifying these days and nobody should have to go in there to understand > > why we select ARCH_HAS_MEMBARRIER_SYNC_CORE. If you really feel a justification > > is needed, I'd be happy with a line in the Kconfig file. > > My concern is that someone wanting to optimize away a few cycles by changing > eret to something else in the future will not be looking at Kconfig: that > person will be staring at entry.S. That person will probably also be me, or somebody who sits within punching distance. I really wouldn't worry about it. There a bunch of other things that will break if we don't use ERET here and, if it's a real concern, we're making the *huge* assumption that developers actually read and pay attention to comments. > One alternative presented by PeterZ on irc is to do like ppc: define a > macro for eret, and stick all appropriate comments near the macro. This > way, it won't hurt when reading the code, but at least it keeps the > comments near the instruction being discussed. For the sake of avoiding the conflict, can we just drop it for now, please? Having an "eret" macro isn't obvious, because people won't realise that it's a macro. Having "exception_return" is cryptic as hell to people familiar with the ISA. Will -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
