Re: [PATCH v5 6/9] ALSA: virtio: PCM substream operators

On 25.02.2021 11:55, Takashi Iwai wrote:

On Mon, 22 Feb 2021 16:34:41 +0100,
Anton Yakovlev wrote:

quoted

+static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
+{
+     struct virtio_pcm *vpcm = snd_pcm_substream_chip(substream);
+     struct virtio_pcm_substream *vss = NULL;
+
+     if (vpcm) {
+             switch (substream->stream) {
+             case SNDRV_PCM_STREAM_PLAYBACK:
+             case SNDRV_PCM_STREAM_CAPTURE: {

The switch() here looks superfluous.  The substream->stream must be a
good value in the callback.  If any, you can put WARN_ON() there, but
I don't think it worth.

At least it doesn't do any harm. If something really went wrong, we can
check it right in the open callback, which is called the very first.

quoted

+static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
+                              struct snd_pcm_hw_params *hw_params)
+{

....

quoted

+     return virtsnd_pcm_msg_alloc(vss, periods, period_bytes);

We have the allocation, but...

quoted

+static int virtsnd_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+     return 0;

... no release at hw_free()?
I know that the free is present in the allocator, but it's only for
re-allocation case, I suppose.

When the substream stops, sync_ptr waits until the device has completed
all pending messages. This wait can be interrupted either by a signal or
due to a timeout. In this case, the device can still access messages
even after calling hw_free(). It can also issue an interrupt, and the
interrupt handler will also try to access message structures. Therefore,
freeing of already allocated messages occurs either in hw_params() or in
dev->release(), since there it is 100% safe.

thanks,

Takashi

-- 
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization