Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Nov 2009 07:43:26 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        Jung-uk Kim <jkim@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Robert Watson <rwatson@freebsd.org>
Subject:   Re: svn commit: r199498 - in head/sys: amd64/amd64 i386/i386 net
Message-ID:  <200911200743.27239.jhb@freebsd.org>
In-Reply-To: <200911191731.04075.jkim@FreeBSD.org>
References:  <200911182340.nAINeJ3W087652@svn.freebsd.org> <200911191649.37198.jhb@freebsd.org> <200911191731.04075.jkim@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 19 November 2009 5:31:00 pm Jung-uk Kim wrote:
> On Thursday 19 November 2009 04:49 pm, John Baldwin wrote:
> > On Thursday 19 November 2009 11:15:01 am Jung-uk Kim wrote:
> > > On Thursday 19 November 2009 03:26 am, Robert Watson wrote:
> > > > On Wed, 18 Nov 2009, Jung-uk Kim wrote:
> > > > >  - Change internal function bpf_jit_compile() to return
> > > > > allocated size of the generated binary and remove page size
> > > > > limitation for userland. - Use contigmalloc(9)/contigfree(9)
> > > > > instead of malloc(9)/free(9) to make sure the generated
> > > > > binary aligns properly and make it physically contiguous.
> > > >
> > > > Is physical contiguity actually required here -- I would have
> > > > thought virtual contiguity and alignment would be sufficient,
> > > > in which case the normal trick is to allocate using malloc the
> > > > size + min-align + 1 and then fudge the pointer forward until
> > > > it's properly aligned.
> > >
> > > I don't believe it is strictly necessary but I assumed it might
> > > have performance benefit for very big BPF programs although I
> > > have not measured it.  Also, contigmalloc(9)/contigfree(9) is too
> > > obvious to ignore for this purpose. :-)
> >
> > Why would it have a performance benefit to have the pages be
> > physically contiguous?  contigmalloc() is expensive and should
> > really only be used if you truly need contiguous memory.  If you
> > can get by with malloc(), just use malloc().
> 
> Remember are allocating memory for a function pointer here.  If we 
> want to take care of alignment, then "fudging the pointer forward" 
> trick is not going to be easy unless I embed real offset in the 
> structure and pass it around with the pointer.  I don't mind doing it 
> but it seemed unnecessary to me.  Besides, it is very unlikely to see 
> a lot of parallel BPF filter allocations in real world.  Actually, 
> that is a big assumption for BPF JIT compiler by itself because 
> filter compilation is expensive.  Also, if contigmalloc() fails, 
> bpf(4) simply falls back to good old bpf_filter().  So, I don't see 
> anything wrong with this.

Why does a function pointer matter?  Fudging the pointer forward will always 
work as virtual addresses always have the same sub-page alignment as physical 
addresses, so doing something like:

	foo *realp;
	void *p;

	align = 16;
	p = malloc(size + (align -1));
	realp = (foo *)(roundup2((uintptr_t)p, align));

Will always work to give a 16-byte aligned pointer.  However, the in-kernel 
malloc() already gives you aligned memory anyway.  Are you seeing any panics
or buggy behavior when using malloc()?

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911200743.27239.jhb>