Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
MIME-Version: 1.0
References: <20231010164234.140750-1-ubizjak@gmail.com> <CAHk-=whYWhZN52SJN-Th9x2L2V-vHtAXUgiy_nSJ3+vQU6ak4Q@mail.gmail.com>
 <CAFULd4ZqH3FeG8_mjDvUAU9QiGB36wDu3MzUtadgAgoVuQ9QRg@mail.gmail.com>
 <CAHk-=wiALZxieQQmvv5sW15HYB_YwC3d_ma9sdp7Zb4Fb4uK2w@mail.gmail.com> <CAFULd4Y8NSArDqH=VMy8F97eNosUUGxrBMEyHH=MytjUBSCmjg@mail.gmail.com>
In-Reply-To: <CAFULd4Y8NSArDqH=VMy8F97eNosUUGxrBMEyHH=MytjUBSCmjg@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 10 Oct 2023 11:52:25 -0700
Message-ID: <CAHk-=whMr8V_q3dq4iS0dpx4Nssu+aYWz+mA36p2ykA+OXTjXA@mail.gmail.com>
Subject: Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()
To:     Uros Bizjak <ubizjak@gmail.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        Nadav Amit <namit@vmware.com>,
        Andy Lutomirski <luto@kernel.org>,
        Brian Gerst <brgerst@gmail.com>,
        Denys Vlasenko <dvlasenk@redhat.com>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 10 Oct 2023 at 11:41, Uros Bizjak <ubizjak@gmail.com> wrote:
>
> Yes, but does it CSE the load from multiple addresses?

Yes, it should do that just right, because the *asm* itself is
identical, just the offsets (that gcc then adds separately) would be
different.

This is not unlike how we depend on gcc CSE'ing the "current" part
when doing multiple accesses of different members off that:

    static __always_inline struct task_struct *get_current(void)
    {
        return this_cpu_read_stable(pcpu_hot.current_task);
    }

with this_cpu_read_stable() being an inline asm that lacks the memory
component (the same way the fallback hides it by just using
"%%gs:this_cpu_off" directly inside the asm, instead of exposing it as
a memory access to gcc).

Of course, I think that with the "__seg_gs" patches, we *could* expose
the "%%gs:this_cpu_off" part to gcc, since gcc hopefully then can do
the alias analysis on that side and see that it can CSE the thing
anyway.

That might be a better choice than __FORCE_ORDER, in fact.

IOW, something like

    static __always_inline unsigned long new_cpu_offset(void)
    {
        unsigned long res;
        asm(ALTERNATIVE(
                        "movq " __percpu_arg(1) ",%0",
                        "rdgsbase %0",
                        X86_FEATURE_FSGSBASE)
                : "=r" (res)
                : "m" (this_cpu_off));
        return res;
    }

would presumably work together with your __seg_gs stuff.

UNTESTED!!

            Linus