Messages in this thread | | | Date | Tue, 26 Jan 2021 08:25:58 +0100 (CET) | From | Guennadi Liakhovetski <> | Subject | Re: [PATCH v2 6/9] ALSA: virtio: PCM substream operators |
| |
One more thing I missed yesterday:
On Mon, 25 Jan 2021, Guennadi Liakhovetski wrote:
> > On Sun, 24 Jan 2021, Anton Yakovlev wrote: > >> Introduce the operators required for the operation of substreams. >> >> Signed-off-by: Anton Yakovlev <anton.yakovlev@opensynergy.com> >> --- >> sound/virtio/Makefile | 3 +- >> sound/virtio/virtio_pcm.c | 5 +- >> sound/virtio/virtio_pcm.h | 2 + >> sound/virtio/virtio_pcm_ops.c | 513 ++++++++++++++++++++++++++++++++++ >> 4 files changed, 521 insertions(+), 2 deletions(-) >> create mode 100644 sound/virtio/virtio_pcm_ops.c > > [snip] > >> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c >> new file mode 100644 >> index 000000000000..19882777fcd6 >> --- /dev/null >> +++ b/sound/virtio/virtio_pcm_ops.c >> @@ -0,0 +1,513 @@ > > [snip] > >> +/** >> + * virtsnd_pcm_release() - Release the PCM substream on the device side. >> + * @substream: VirtIO substream. >> + * >> + * Context: Any context that permits to sleep. >> + * Return: 0 on success, -errno on failure. >> + */ >> +static inline bool virtsnd_pcm_released(struct virtio_pcm_substream >> *substream) >> +{ >> + /* >> + * The spec states that upon receipt of the RELEASE command "the >> device >> + * MUST complete all pending I/O messages for the specified stream >> ID". >> + * Thus, we consider the absence of I/O messages in the queue as an >> + * indication that the substream has been released. >> + */ >> + return atomic_read(&substream->msg_count) == 0; > > Also here having it atomic doesn't really seem to help. This just means, that > at some point of time it was == 0. > >> +} >> + >> +static int virtsnd_pcm_release(struct virtio_pcm_substream *substream) > > kernel-doc missing > >> +{ >> + struct virtio_snd *snd = substream->snd; >> + struct virtio_snd_msg *msg; >> + unsigned int js = msecs_to_jiffies(msg_timeout_ms); >> + int rc; >> + >> + msg = virtsnd_pcm_ctl_msg_alloc(substream, VIRTIO_SND_R_PCM_RELEASE, >> + GFP_KERNEL); >> + if (IS_ERR(msg)) >> + return PTR_ERR(msg); >> + >> + rc = virtsnd_ctl_msg_send_sync(snd, msg); >> + if (rc) >> + return rc; >> + >> + return wait_event_interruptible_timeout(substream->msg_empty, >> + virtsnd_pcm_released(substream), >> + js);
wait_event_interruptible_timeout() will return a positive number in success cases, 0 means a timeout and condition still false. Whereas when you call this function you interpret 0 as success and you expect any != 0 to be a negative error. Wondering how this worked during your tests?
Thanks Guennadi
| |