Hi,
On Oct 3 2018 19:30, Takashi Iwai wrote:
On Wed, 03 Oct 2018 10:12:34 +0200,
On Sep 21 2018 00:54, Takashi Iwai wrote:
Per popular demands, this patch adds a new ALSA core API function,
snd_devm_card_new(), to create a snd_card object in a managed way via
devres. When a card object is created by this new function, it's
released automatically at the device release. It includes also the
call of snd_card_free().
However, the story isn't that simple. A caveat is that We have to
call snd_card_new(), more specifically, the disconnection part, at
very first of the whole resource release procedure. This assures that
the exposed devices are deleted and sync with the all accessing
processes getting closed.
For achieving it, snd_card_register() adds a new devres action to
trigger snd_card_free() automatically when the given card object is a
"managed" one. Since usually snd_card_register() is the last step of
the initialization, this should work in most cases.
With all these tricks, some drivers can get rid of the whole the
driver remove callback.
About a bit of implementation details: the patch adds two new flags to
snd_card object, managed and releasing. The former indicates that the
object was created via snd_devm_card_new(), and the latter is used for
avoiding the double-free of snd_card_free() calls. Both flags are
fairly internal and likely uninteresting to normal users.
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
include/sound/core.h | 5 +++
sound/core/init.c | 95 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 96 insertions(+), 4 deletions(-)
I've investigated to use the new function, 'snd_devm_card_new()', and
have a concern about timing to release memory object for sound card
instance (and tailing private data) in error path between calls of the
function and 'snd_card_register()'.
In the error path, 'snd_card_free()' is called to release instances
associated to the sound card instance as expected, however memory object
for the sound card instance is not released yet because in usual cases
associated device structure does not lost its reference count in this
timing. It's released when the associated device is removed. This means
that the memory object remains against its practicality during lifetime
of the device.
This is not a bug itself, so I have no opposition to this patchset. But
it's reasonable for us to have a bit time to consider about it, IMO.
Well, it's exactly same as the usual devm_kzalloc() & co, which have
been already deployed at various places (even you posted a new patch
series for using them :) The card memory is tied with the device, and
it's natural to align with the lifetime of the device.
I have this concern against the usage of devres for a long time.
A device can be referred by several handlers; e.g. character device
driver and kernel driver. An example is USB devices. Userspace
applications can transfer message to the device via character device.
At the same time, iface driver in kernel land can do the same work.
The lifetime of device is apparently different from each of the handler.
Even if kernel driver returns negative value in its .probe callback, the
other handlers can refer to and use it.
If the kernel driver leave the devm-allocated memory in error path, it
remains till all of the handler release its reference to the device.
Even it the memory objects are maintained by devres, several small parts
of kernel logical space is unavailable. This is similar to memory leak,
in my view.
Regards
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
