Messages in this thread | | | Date | Sat, 10 Jul 1999 10:07:41 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: New kernel/resource.c |
| |
On Sat, 10 Jul 1999, Mikael Pettersson wrote: > > 2.3.11-pre1 works fine here, but I noticed two problems: > > 1. The legacy support stuff leaks memory. Both request_region() > and check_region() will kmalloc resource structs which are > never kfree'd.
I'm actually trying to get people to move away from the legacy support. The reason I made request_region() return the region it got is that I personally want drivers to move into doing things like
resource = request_region(a,b,"driver"); ... release_resource(resource);
instead of using "release_region()".
The reason? "release_region()" is a fundamentally broken interface, and cannot handle the nested resources etc, and doesn't have the nice sanity-checks that a release_resource() thing can have (ie validating that it gets passed the same pointer that it got allocated etc).
> The patch below fixes this by adding a kfree() to __check_region(), > having __request_region() set a flag RSRC_AUTOFREE in the > resource struct, and letting __release_region() kfree the > struct if this flag is set. This way, legacy drivers will > continue to work without leaking memory.
I want the "flags" field to be completely anonymous, ie usable for the thing that uses the resource. The allocator and de-allocator should not look at them.
The "flags" stuff is for things like MTRR etc, where each region can have it's own attributes (cachable, write-combine, etc etc). And region attributes depend on what kind of region it is, which is why I don't want to have the generic resource code really know about it.
release_region() should probably just unconditionally kfree() the area - it's only legal to do a release_region() on something that did a request_region() anyway, so the unconditional kfree() is always correct.
> 2. drivers/video/vgacon.c calls request_region() before the > slab allocator and kmalloc are initialised, causing request_region > to silently fail and a "NULL ptr" message in the kernel log. > The patch below fixes this (for vgacon only) by transparently > translating its request_region() calls to request_resource() > calls with static parameters. (The patch (ab)uses the pre- > processor. I kind of like it, but others may find it gross.)
I'd prefer to rename it, so that people don't by mistake think that it does a "real" request_region(). But other than that I don't think it is gross - although I wonder if vgacon.c shouldn't just be using the native stuff directly, the way I made the architecture setup files do it.
(The setup.c code actually got a lot _cleaner_ by using the new native primitives - it's not as if they are ugly or anything)
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |