Re: [PATCH 02/51] ALSA: core: Add managed card creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

...

/**
   * 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);
  	return 0;
  }
@@ -539,6 +607,9 @@ int snd_card_free(struct snd_card *card)
  	DECLARE_COMPLETION_ONSTACK(released);
  	int ret;
+ 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

  	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?

+	}
+
+	if (card->managed) {
+		err = devm_add_action(card->dev, trigger_card_free, card);
+		if (err < 0)
+			return err;
  	}
err = snd_device_register_all(card);





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux