While experimenting with the agp(4) interface I found that if you
release the interface (AGPIOC_RELEASE) before closing its file
descriptor you'll end up with allocated but unbinded memory blocks.
This behavior is due to the fact that the agp_release_helper() function
doesn't free the memory associated to each block and this is incoherent
with what it says it does:
/*
* Clear out the aperture and free any
* outstanding memory blocks.
*/
...
So the diff below correct this by freeing all the memory block when
releasing the interface, this is what happens currently if you close
the file descriptor without releasing the interface.
However it doesn't change the behavior of the interface that looks
busted to me because we don't keep track of who allocated some memory
block and end up unbindind and freeing everything.
Ok?
Index: agp.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/agp.c,v
retrieving revision 1.34
diff -u -p -r1.34 agp.c
--- agp.c 26 Dec 2010 15:41:00 -0000 1.34
+++ agp.c 12 Nov 2012 18:20:48 -0000
@@ -290,20 +310,13 @@ int
agpclose(dev_t dev, int flags, int devtype, struct proc *p)
{
struct agp_softc *sc = agp_find_device(AGPUNIT(dev));
- struct agp_memory *mem;
/*
* Clear the GATT and force release on last close
*/
- if (sc->sc_state == AGP_ACQUIRE_USER) {
- while ((mem = TAILQ_FIRST(&sc->sc_memory)) != 0) {
- if (mem->am_is_bound) {
- agp_unbind_memory(sc, mem);
- }
- agp_free_memory(sc, mem);
- }
+ if (sc->sc_state == AGP_ACQUIRE_USER)
agp_release_helper(sc, AGP_ACQUIRE_USER);
- }
+
sc->sc_opened = 0;
return (0);
@@ -658,25 +671,26 @@ int
agp_release_helper(void *dev, enum agp_acquire_state state)
{
struct agp_softc *sc = (struct agp_softc *)dev;
- struct agp_memory* mem;
+ struct agp_memory *mem, *tmp;
if (sc->sc_state == AGP_ACQUIRE_FREE)
return (0);
- if (sc->sc_state != state)
+ if (sc->sc_state != state)
return (EBUSY);
/*
* Clear out the aperture and free any
* outstanding memory blocks.
*/
- TAILQ_FOREACH(mem, &sc->sc_memory, am_link) {
+ TAILQ_FOREACH_SAFE(mem, &sc->sc_memory, am_link, tmp) {
if (mem->am_is_bound) {
- printf("agp_release_helper: mem %d is bound\n",
- mem->am_id);
+ AGP_DPF("mem %d is bound\n", mem->am_id);
agp_unbind_memory(sc, mem);
}
+ agp_free_memory(sc, mem);
}
+
sc->sc_state = AGP_ACQUIRE_FREE;
return (0);
}
