Re: [patch 1/2] agpgart - allow user-populated memory types.

[Arjan van de Ven <arjan@infradead.org>]

On Sat, 2006-12-09 at 00:05 +0100, Thomas Hellström wrote:
> > On Fri, 2006-12-08 at 19:24 +0100, Thomas Hellström wrote:
> >>
> >> +       }
> >> +
> >> +       if (alloc_size <= PAGE_SIZE) {
> >> +               new->memory = kmalloc(alloc_size, GFP_KERNEL);
> >> +       }
> >> +       if (new->memory == NULL) {
> >> +               new->memory = vmalloc(alloc_size);
> >
> > this bit is more or less evil as well...
> >
> > 1) vmalloc is expensive all the way, higher tlb use etc etc
> > 2) mixing allocation types is just a recipe for disaster
> > 3) if this isn't a frequent operation, kmalloc is fine upto at least 2
> > pages; I doubt you'll ever want more
> 
> I understand your feelings about this, and as you probably understand, the
> kfree / vfree thingy is a result of the above allocation scheme.

the kfree/vfree thing at MINIMUM should be changed though. Even if you
need both kfree and vfree, you should key it off of a flag that you
store, not off the address of the memory, that's just unportable and
highly fragile. You *know* which allocator you used, so store it and use
THAT info.



> The allocated memory holds an array of struct page pointers. The number of
> struct page pointers will range from 1 to about 8192, so the alloc size
> will range from 4bytes to 64K, but could go higher depending on
> architecture.

hmm 64Kb is a bit much indeed. You can't do an array of upto 16 entries
with one page in each array entry? 

>  I figured that kmalloc could fail, and, according to "linux
> device drivers" (IIRC), vmalloc is faster when allocation size exceeds 1
> page.

that is just plain incorrect. Vmalloc is really really slow, both in
allocating and in use. During use you trash the 4Kb tlbs all the time
for example, and that's just cost that accumulates.

> So we can't use only vmalloc, and we can't fail so we can't use only
> kmalloc. I'm open to suggestions on how to improve this.

cleanest is probably the array of pages thing. to index it then you
first calculate which entry to use (that's half a cycle since it's a
power of 2) and then you index inside that page... that's going to be
faster than the tlb miss no doubt.

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/