[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z1ysj_PXy51WeAT2@krava>
Date: Fri, 13 Dec 2024 22:52:15 +0100
From: Jiri Olsa <olsajiri@...il.com>
To: Thomas Weißschuh <thomas@...ch.de>
Cc: Jiri Olsa <olsajiri@...il.com>, Oleg Nesterov <oleg@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
Hao Luo <haoluo@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Alan Maguire <alan.maguire@...cle.com>,
linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next 06/13] uprobes/x86: Add uprobe syscall to speed
up uprobe
On Fri, Dec 13, 2024 at 04:12:46PM +0100, Thomas Weißschuh wrote:
SNIP
> > > > +static int __init arch_uprobes_init(void)
> > > > +{
> > > > + unsigned long size = uprobe_trampoline_end - uprobe_trampoline_entry;
> > > > + static struct page *pages[2];
> > > > + struct page *page;
> > > > +
> > > > + page = alloc_page(GFP_HIGHUSER);
> > >
> > > That page could be in static memory, removing the need for the explicit
> > > allocation. It could also be __ro_after_init.
> > > Then tramp_mapping itself can be const.
> >
> > hum, how would that look like? I think that to get proper page object
> > you have to call alloc_page or some other page alloc family function..
> > what do I miss?
>
> static u8 trampoline_page[PAGE_SIZE] __ro_after_init __aligned(PAGE_SIZE);
> static struct page *tramp_mapping_pages[2] __ro_after_init;
>
> static const struct vm_special_mapping tramp_mapping = {
> .name = "[uprobes-trampoline]",
> .pages = tramp_mapping_pages,
> .mremap = tramp_mremap,
> };
>
> static int __init arch_uprobes_init(void)
> {
> ...
> trampoline_pages[0] = virt_to_page(trampoline_page);
> ...
> }
>
> Untested, but it's similar to the stuff the vDSO implementations are
> doing which I am working with at the moment.
nice idea, better than allocating the page, will do that
>
> > >
> > > Also this seems to waste the page on 32bit kernels.
> >
> > it's inside CONFIG_X86_64 ifdef
> >
> > >
> > > > + if (!page)
> > > > + return -ENOMEM;
> > > > + pages[0] = page;
> > > > + tramp_mapping.pages = (struct page **) &pages;
> > >
> > > tramp_mapping.pages = pages; ?
> >
> > I think the compiler will cry about *pages[2] vs **pages types mismatch,
> > but I'll double check that
>
> It compiles for me.
ok
thanks,
jirka
Powered by blists - more mailing lists