Re: [PATCH RFC v2 01/29] mm: asi: Make some utility functions noinstr compatible
Brendan Jackman <[email protected]>
| Newsgroups | org.kernel.vger.linux-hexagon,dev.linux.lists.loongarch,org.infradead.lists.linux-riscv,org.infradead.lists.linux-snps-arc,org.infradead.lists.linux-um,org.kernel.vger.kvm,org.kernel.vger.linux-alpha,org.kernel.vger.linux-arch,org.kernel.vger.linux-csky,org.kernel.vger.linux-efi,org.kernel.vger.linux-kernel,org.kernel.vger.linux-m68k,org.kernel.vger.linux-mips,org.kernel.vger.linux-openrisc,org.kernel.vger.linux-parisc,org.kernel.vger.linux-perf-users,org.kernel.vger.linux-s390,org.kernel.vger.linux-sh,org.kernel.vger.linux-trace-kernel,org.kernel.vger.sparclinux,org.kvack.linux-mm,org.ozlabs.lists.linuxppc-dev |
|---|---|
| Message-ID | <CA+i-1C1sXC1tbo9ytuex0eBD3gUbQwnNP8rvOhCv=JV+oSHo1g@mail.gmail.com> |
On Thu, 16 Jan 2025 at 01:21, Borislav Petkov <[email protected]> wrote: > > Unfortunately Thomas pointed out this will prevent the function from > > being inlined at call sites in .text. > > > > So far I haven't been able[1] to find a formulation that lets us : > > 1. avoid calls from .noinstr.text -> .text, > > 2. while also letting the compiler freely decide what to inline. > > > > 1 is a functional requirement so here I'm just giving up on 2. Existing > > callsites of this code are just forced inline. For the incoming code > > that needs to call it from noinstr, they will be out-of-line calls. > > I'm not sure some of that belongs in the commit message - if you want to have > it in the submission, you should put it under the --- line below, right above > the diffstat. Sure. I'm actually not even sure that for a [PATCH]-quality thing this cross-cutting commit even makes sense - once we've decided on the general way to solve this problem, perhaps the changes should just be part of the commit that needs them? It feels messy to have a patch that "does multiple things", but on the other hand it might be annoying to review a patch that says "make a load of random changes across the kernel, which are needed at various points in various upcoming patches, trust me". Do you have any opinion on that? (BTW, since a comment you made on another series (can't find it on Lore...), I've changed my writing style to avoid stuff like this in comments & commit messages in general, but this text all predates that. I'll do my best to sort all that stuff out before I send anything as a [PATCH].) On Thu, 16 Jan 2025 at 11:29, Borislav Petkov <[email protected]> wrote: > > On Thu, Jan 16, 2025 at 01:18:58AM +0100, Borislav Petkov wrote: > > Long story short, lemme try to poke around tomorrow to try to figure out what > > actually happens. It could be caused by the part of Rik's patches and this one > > inlining things. We'll see... > > Looks transient... The very similar guest boots fine on another machine. Let's > watch this... Oh, I didn't notice your update until now. But yeah I also couldn't reproduce it on a Sapphire Rapids machine and on QEMU with this patch applied on top of tip/master (37bc915c6ad0f).