On Mon, Nov 22, 2021 at 3:19 PM Sai Prakash Ranjan <quic_saipraka@xxxxxxxxxxx> wrote: > On 11/22/2021 7:29 PM, Arnd Bergmann wrote: > > > > I think this would be a lot less confusing to readers, as it is implemented > > exactly in the place that has the normal definition, and it can also have > > somewhat more logical semantics by only instrumenting the > > normal/relaxed/ioport accessors but not the __raw_* versions that > > are meant to be little more than a pointer dereference. > > But how is this different from logic in atomic-instrumented.h which also > has asm-generic version? > Initial review few years back mentioned about having something similar > to atomic instrumentation > and hence it was implemented with the similar approach keeping > instrumentation out of arch specific details. This is only a cosmetic difference. I usually prefer fewer indirections, and I like the way that include/asm-generic/io.h only has all the normal 'static inline' definitions spelled out, and calling the __raw_* versions. Your version adds an extra layer with the arch_raw_readl(), which I'd prefer to avoid. > And if we do move this instrumentation to asm-generic/io.h, how will > that be executed since > the arch specifc read{b,w,l,q} overrides this generic version? As I understand it, your version also requires architecture specific changes, so that would be the same: it only works for architectures that get the definition of readl()/readl_relaxed()/inl()/... from include/asm-generic/io.h and only override the __raw version. Arnd