Re: [PATCH 01/26] mm: asi: Make some utility functions noinstr compatible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/1/24 1:27 PM, Thomas Gleixner wrote:
On Thu, Oct 31 2024 at 18:44, Junaid Shahid wrote:
On 10/29/24 12:12 PM, Thomas Gleixner wrote:

I doubt that it works as you want it to work.

+	inline notrace __attribute((__section__(".noinstr.text")))	\

So this explicitely puts the inline into the .noinstr.text section,
which means when it is used in .text the compiler will generate an out-of
line function in the .noinstr.text section and insert a call into the
usage site. That's independent of the size of the inline.


Oh, that's interesting. IIRC I had seen regular (.text) inline functions get
inlined into .noinstr.text callers. I assume the difference is that here the
section is marked explicitly rather than being implicit?

Correct. Inlines without any section attribute are free to be inlined in
any section, but if the compiler decides to uninline them, then it
sticks the uninlined version into the default section ".text".

The other problem there is that an out of line version can be
instrumented if not explicitely forbidden.

That's why we mark them __always_inline, which forces the compiler to
inline it into the usage site unconditionally.

In any case, I guess we could just mark these functions as plain
noinstr.

No. Some of them are used in hotpath '.text'. 'noinstr' prevents them to
be actually inlined then as I explained to you before.

(Unless there happens to be some other way to indicate to the compiler to place
any non-inlined copy of the function in .noinstr.text but still allow inlining
into .text if it makes sense optimization-wise.)

Ideally the compilers would provide

         __attribute__(force_caller_section)

which makes them place an out of line inline into the section of the
function from which it is called. But we can't have useful things or
they are so badly documented that I can't find them ...

What actually works by some definition of "works" is:

        static __always_inline void __foo(void) { }

        static inline void foo(void)
        {
                 __(foo);
        }

        static inline noinstr void foo_noinstr(void)
        {
                 __(foo);
        }

The problem is that both GCC and clang optimize foo[_noinstr]() away and
then follow the __always_inline directive of __foo() even if I make
__foo() insanely large and have a gazillion of different functions
marked noinline invoking foo() or foo_noinstr(), unless I add -fno-inline
to the command line.

Which means it's not much different from just having '__always_inline
foo()' without the wrappers....

Compilers clearly lack a --do-what-I-mean command line option.

Now if I'm truly nasty then both compilers do what I mean even without a
magic command line option:

        static __always_inline void __foo(void) { }

        static __maybe_unused void foo(void)
        {
                 __(foo);
        }

        static __maybe_unused noinstr void foo_noinstr(void)
        {
                 __(foo);
        }

If there is a single invocation of either foo() or foo_noinstr() and
they are small enough then the compiler inlines them, unless -fno-inline
is on the command line. If there are multiple invocations and/or foo
gets big enough then both compilers out of line them. The out of line
wrappers with __foo() inlined in them end always up in the correct
section.

I actually really like the programming model as it is very clear about
the intention of usage and it allows static checkers to validate.

Thoughts?


Thank you for the details. Yes, I think the last scheme that you described with separate wrappers for regular and noinst usage makes sense. IIRC the existing static validation wouldn't catch it if someone mistakenly called the .text version of the function from noinstr code and it just happened to get inlined. Perhaps we should add the -fno-inline compiler option with CONFIG_NOINSTR_VALIDATION?

Thanks,
Junaid






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux