* Song Liu <songliubraving@xxxxxx> [2019-10-17 09:42:22]:
> Attaching uprobe to text section in THP splits the PMD mapped page table
> into PTE mapped entries. On uprobe detach, we would like to regroup PMD
> mapped page table entry to regain performance benefit of THP.
>
> However, the regroup is broken For perf_event based trace_uprobe. This is
> because perf_event based trace_uprobe calls uprobe_unregister twice on
> close: first in TRACE_REG_PERF_CLOSE, then in TRACE_REG_PERF_UNREGISTER.
> The second call will split the PMD mapped page table entry, which is not
> the desired behavior.
>
> Fix this by only use FOLL_SPLIT_PMD for uprobe register case.
>
> Add a WARN() to confirm uprobe unregister never work on huge pages, and
> abort the operation when this WARN() triggers.
>
> Fixes: 5a52c9df62b4 ("uprobe: use FOLL_SPLIT_PMD instead of FOLL_SPLIT")
> Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Srikar Dronamraju <srikar@xxxxxxxxxxxxxxxxxx>
> Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
> Signed-off-by: Song Liu <songliubraving@xxxxxx>
> ---
Looks good to me.
Reviewed-by: Srikar Dronamraju <srikar@xxxxxxxxxxxxxxxxxx>
> kernel/events/uprobes.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 94d38a39d72e..c74761004ee5 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -474,14 +474,17 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
> struct vm_area_struct *vma;
> int ret, is_register, ref_ctr_updated = 0;
> bool orig_page_huge = false;
> + unsigned int gup_flags = FOLL_FORCE;
>
> is_register = is_swbp_insn(&opcode);
> uprobe = container_of(auprobe, struct uprobe, arch);
>
> retry:
> + if (is_register)
> + gup_flags |= FOLL_SPLIT_PMD;
> /* Read the page with vaddr into memory */
> - ret = get_user_pages_remote(NULL, mm, vaddr, 1,
> - FOLL_FORCE | FOLL_SPLIT_PMD, &old_page, &vma, NULL);
> + ret = get_user_pages_remote(NULL, mm, vaddr, 1, gup_flags,
> + &old_page, &vma, NULL);
> if (ret <= 0)
> return ret;
>
> @@ -489,6 +492,12 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
> if (ret <= 0)
> goto put_old;
>
> + if (WARN(!is_register && PageCompound(old_page),
> + "uprobe unregister should never work on compound page\n")) {
> + ret = -EINVAL;
> + goto put_old;
> + }
> +
> /* We are going to replace instruction, update ref_ctr. */
> if (!ref_ctr_updated && uprobe->ref_ctr_offset) {
> ret = update_ref_ctr(uprobe, mm, is_register ? 1 : -1);
> --
> 2.17.1
>
--
Thanks and Regards
Srikar Dronamraju
