Thread (4 messages) 4 messages, 3 authors, 2025-01-31

Re: [PATCH] virtio_console: Convert to use devm funcs

From: "Arnd Bergmann" <arnd@arndb.de>
Date: 2025-01-31 07:49:03
Also in: lkml

On Wed, Jan 29, 2025, at 12:48, oushixiong wrote:
在 2025/1/28 17:39, Amit Shah 写道:
quoted
On Tue, 2025-01-28 at 13:52 +0800, oushixiong1025@163.com wrote:

Hm, I'm not entirely sure about this - the devm_ interface is better,
but to me that just says that it's an extra safety net that avoids
memleaks when we forget to kfree, and not that we deliberately do not
free and get lax about managing allocated memory.

So I'd prefer a patch that keeps all current frees as they are, but
with the added advantage of using the devm interfaces.
Hi, I don't think driver need to  call kfree() by manual if driver use 
the devm_ interface
Calling the plain kfree() on memory allocated by devm_kmalloc()
is a bug, it has to be freed when the device is released or
explicitly using devm_kfree(). The explicit devm_kfree() in
turn is what you'd use only if memory has to be freed early
while the device is still going to be used.
in the probe() phase. This can simplifies the code.

This is similar to the  commit  "55dd7378ba94 (fbdev: imxfb: Convert to 
devm_kmalloc_array())"
I don't think it's worth converting drivers that correctly use
kmalloc()/kfree() already. For new drivers, the devm_* functions
are usually the easiest to use and review, and it makes sense to
convert drivers that have known memory leaks from incorrect
cleanup paths, but I don't think it's a good use of developer
or reviewer time to convert drivers that are not wrong for the
sake of simplicity. There is a memory overhead in the devm_*
functions and there is a risk of introducing bugs if the conversion
is done incorrectly, e.g. when a regular kfree() is left behind,
or when changing allocations that are meant to be freed before
.release() is called.

       Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help