Messages in this thread | | | Date | Sun, 26 Apr 2009 18:08:59 +0200 | From | Ingo Molnar <> | Subject | Re: [rfc 2/2] x86, bts: use physically non-contiguous trace buffer |
| |
* Metzger, Markus T <markus.t.metzger@intel.com> wrote:
> >-----Original Message----- > >From: Ingo Molnar [mailto:mingo@elte.hu] > >Sent: Friday, April 24, 2009 10:31 AM > >To: Andrew Morton > >Cc: Metzger, Markus T; a.p.zijlstra@chello.nl; markus.t.metzger@gmail.com; roland@redhat.com; > >eranian@googlemail.com; oleg@redhat.com; Villacis, Juan; ak@linux.jf.intel.com; linux- > >kernel@vger.kernel.org; tglx@linutronix.de; hpa@zytor.com > >Subject: Re: [rfc 2/2] x86, bts: use physically non-contiguous trace buffer > > > > > >* Andrew Morton <akpm@linux-foundation.org> wrote: > > > >> On Fri, 24 Apr 2009 10:00:55 +0200 Markus Metzger <markus.t.metzger@intel.com> wrote: > >> > >> > Use vmalloc to allocate the branch trace buffer. > >> > > >> > Peter Zijlstra suggested to use vmalloc rather than kmalloc to > >> > allocate the potentially multi-page branch trace buffer. > >> > >> The changelog provides no reason for this change. It should do so. > >> > >> > Is there a way to have vmalloc allocate a physically non-contiguous > >> > buffer for test purposes? Ideally, the memory area would have big > >> > holes in it with sensitive data in between so I would know immediately > >> > when this is overwritten. > >> > >> I suppose you could allocate the pages by hand and then vmap() them. > >> Allocating 2* the number you need and then freeing every second one > >> should make them physically holey. > >> > >> > --- a/arch/x86/kernel/ptrace.c > >> > +++ b/arch/x86/kernel/ptrace.c > >> > @@ -22,6 +22,7 @@ > >> > #include <linux/seccomp.h> > >> > #include <linux/signal.h> > >> > #include <linux/workqueue.h> > >> > +#include <linux/vmalloc.h> > >> > > >> > #include <asm/uaccess.h> > >> > #include <asm/pgtable.h> > >> > @@ -626,7 +627,7 @@ static int alloc_bts_buffer(struct bts_c > >> > if (err < 0) > >> > return err; > >> > > >> > - buffer = kzalloc(size, GFP_KERNEL); > >> > + buffer = vmalloc(size); > >> > if (!buffer) > >> > goto out_refund; > >> > > >> > @@ -646,7 +647,7 @@ static inline void free_bts_buffer(struc > >> > if (!context->buffer) > >> > return; > >> > > >> > - kfree(context->buffer); > >> > + vfree(context->buffer); > >> > context->buffer = NULL; > >> > > >> > >> The patch looks like a regression to me. vmalloc memory is slower > >> to allocate, slower to free, slower to access and can exhaust or > >> fragment the vmalloc arena. Confused. > > > >Performance does not matter here (this is really a slowpath), but > >fragmentation does matter, especially on 32-bit systems. > > > >I'd not uglify the code via vmap() - and vmap has the same > >fundamental address space limitations on 32-bit as vmalloc(). > > > >The existing kmalloc() is fine. We do larger than PAGE_SIZE > >allocations elsewhere too (the kernel stack for example), and this > >is a debug facility, so failing the allocation is not a big problem > >even if it happens. > > OK. I'll drop 2/2 and send out 1/2 as a patch, then.
ok - i've already applied 1/2 so unless you can see a bug we should be fine.
> The original suggestion was to use the page allocator and vmap(). > I assume you don't want that, either.
Yeah - i'd rather suggest to avoid that complexity - unless there are good reasons.
Ingo
| |