Messages in this thread | | | From | Kay Sievers <> | Date | Tue, 27 Apr 2010 15:34:15 +0200 | Subject | Re: request_firmware API exhaust memory |
| |
On Tue, Apr 27, 2010 at 14:43, David Woodhouse <dwmw2@infradead.org> wrote: > On Tue, 2010-04-27 at 14:05 +0200, Kay Sievers wrote: >> >> The patch I posted makes the issue go away. It's still not the right >> fix, because the pages are only get freed when the device id cleaned >> up, not on calling release_firmware. But it should illustrate the >> underlying issue, and that there is no leaked memory anymore. >> >> > I think this needs some more review. >> >> If David does not fix it, it probably just needs to be reverted. And >> instead of implementing our own "memory management", we should rather >> add a vrealloc(), and the firmware loader should use that. > > The whole point was to avoid the vrealloc(). We really don't want to be > screwing with page tables, globally, for each page written from > userspace.
Yeah. I guess the problem with the old code before you made if faster was that it was doing vmalloc()->memcpy()->vfree() in a loop. A vrealloc(), which we don't have today, could be made reasonable fast, I guess.
> This untested patch attempts to put the page array into the 'struct > firmware' so that we can free it from release_firmware().
Looks good. Seems to work without problems and without leaking memory.
Misses only the member in the struct firmware though. :)
> It would actually be nice if we could make that the _primary_ method of > returning data to drivers, and we could ditch the vmap() requirement > altogether... drivers which really need it to be virtually contiguous > can depend on CONFIG_MMU and do the vmap() for themselves.
Yeah, we could just do that in a firmware_get_data() accessor function and call vmap() there on-demand. That way we would just account and copy pages in the firmware class until the data is actually accessed. Existing users would just need to change the direct ->data access to firmware_get_data().
Thanks, Kay -- 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/
| |