Re: [patch 1/2] x86/mm/pti: Handle unaligned address gracefully in pti_clone_pagetable()

[Thomas Gleixner]

On Wed, 28 Aug 2019, Song Liu wrote:
> > On Aug 28, 2019, at 8:51 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > 
> > On Wed, 28 Aug 2019, Dave Hansen wrote:
> >> On 8/28/19 7:24 AM, Thomas Gleixner wrote:
> >>> From: Song Liu <songliubraving@xxxxxx>
> >>> 
> >>> pti_clone_pmds() assumes that the supplied address is either:
> >>> 
> >>> - properly PUD/PMD aligned
> >>> or
> >>> - the address is actually mapped which means that independent
> >>>   of the mapping level (PUD/PMD/PTE) the next higher mapping
> >>>   exist.
> >>> 
> >>> If that's not the case the unaligned address can be incremented by PUD or
> >>> PMD size wrongly. All callers supply mapped and/or aligned addresses, but
> >>> for robustness sake, it's better to handle that case proper and to emit a
> >>> warning.
> >> 
> >> Reviewed-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> >> 
> >> Song, did you ever root-cause the performance regression?  I thought
> >> there were still some mysteries there.
> > 
> > See Peter's series to rework the ftrace code patching ...
> 
> Thanks Thomas. 
> 
> Yes, in summary, enabling ftrace or kprobe-on-ftrace causes the kernel
> to split PMDs in kernel text mapping. 
> 
> Related question: while Peter's patches fix it for 5.3 kernel, they don't 
> apply cleanly over 5.2 kernel (which we are using). So I wonder what is
> the best solution for 5.2 kernel. May patch also fixes the issue:
> 
> https://lore.kernel.org/lkml/20190823052335.572133-1-songliubraving@xxxxxx/
> 
> How about we apply this patch to upstream 5.2 kernel?

That's not how it works. We fix stuff upstream and it gets backported to
all affected kernels not just to the one you care about.

Aside of that I really disagree with that hack. You completely fail to
explain why that commit in question broke it and instead of fixing the
underlying issue you create a horrible workaround.

It took me ~10 minutes to analyze the root cause and I'm just booting the
test box with a proper fix which can be actually tagged for stable and can
be removed from upstream again once ftrace got moved over to text poke.

I'll post it once it's confirmed to work and I wrote a comprehensible
changelog.

Thanks,

	tglx