On Tue, 13 Jul 2021 17:23:02 +0200, Amadeusz SX2awiX4ski wrote:
On 7/13/2021 4:28 PM, Takashi Iwai wrote:
...
+/**
- snd_devm_card_new - managed snd_card object creation
- @parent: the parent device object
- @idx: card index (address) [0 ... (SNDRV_CARDS-1)]
- @xid: card identification (ASCII string)
- @module: top level module for locking
- @extra_size: allocate this extra size after the main soundcard structure
- @card_ret: the pointer to store the created card instance
- This function works like snd_card_new() but manages the allocated resource
- via devres, i.e. you don't need to free explicitly.
- When a snd_card object is created with this function and registered via
- snd_card_register(), the very first devres action to call snd_card_free()
- is added automatically. In that way, the resource disconnection is assured
- at first, then released in the expected order.
- */
+int snd_devm_card_new(struct device *parent, int idx, const char *xid,
struct module *module, int extra_size,struct snd_card **card_ret)+{
- struct snd_card *card;
- int err;
- *card_ret = NULL;
- if (extra_size < 0)
extra_size = 0;Maybe just make extra_size unsigned or even better size_t?
OK, that would fit for a new function, indeed. Will modify in v2.
/**
- snd_card_ref - Get the card object from the index
@@ -481,6 +547,7 @@ EXPORT_SYMBOL_GPL(snd_card_disconnect_sync); static int snd_card_do_free(struct snd_card *card) {
- card->releasing = true; #if IS_ENABLED(CONFIG_SND_MIXER_OSS) if (snd_mixer_oss_notify_callback) snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE);
@@ -498,7 +565,8 @@ static int snd_card_do_free(struct snd_card *card) #endif if (card->release_completion) complete(card->release_completion);
- kfree(card);
- if (!card->managed)
kfree(card);
- if (card->releasing)
return 0;"card->releasing" use feels bit racy to me... something like below would break it?
thread1 thread2 snd_card_free() if(card->releasing) == false thread1 goes sleep snd_card_do_free() card->releasing = true run until the end thread1 resume continues with trying to release
It's a destructor and can't be called in parallel.
card->release_completion = &released; ret = snd_card_free_when_closed(card); if (ret) @@ -745,6 +816,11 @@ int snd_card_add_dev_attr(struct snd_card *card, } EXPORT_SYMBOL_GPL(snd_card_add_dev_attr); +static void trigger_card_free(void *data) +{
- snd_card_free(data);
+}
- /**
- snd_card_register - register the soundcard
- @card: soundcard structure
@@ -768,6 +844,15 @@ int snd_card_register(struct snd_card *card) if (err < 0) return err; card->registered = true;
- } else {
if (card->managed)devm_remove_action(card->dev, trigger_card_free, card);Not sure I understand, we are in _register function, so why do we remove action?
snd_card_register() can be called multiple times for re-registering the newly added components (e.g. usb-audio driver does it). In that case, we have to move this trigger_card_free action at the head again.
thanks,
Takashi