lkml.org 
[lkml]   [2010]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: 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/

\
 
 \ /
  Last update: 2010-04-27 15:37    [W:0.088 / U:0.952 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site