Re: New kernel/resource.c

Linus Torvalds (torvalds@transmeta.com)
Sat, 10 Jul 1999 10:07:41 -0700 (PDT)


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/