[PATCH 0/2] ALSA: compress: clean up pointer errno handling and fix avail errors

[PATCH 0/2] ALSA: compress: clean up pointer errno handling and fix avail errors

[Cássio Gabriel]

This series tightens compressed-offload pointer error handling in two
small steps.

Patch 1 switches the internal "pointer callback not implemented" case
from ENOTSUPP to EOPNOTSUPP so the code uses the standard unsupported
operation errno.

Patch 2 then propagates real pointer-refresh failures through the avail
paths while keeping the unsupported-pointer fallback non-fatal. This
prevents SNDRV_COMPRESS_AVAIL, read(), write(), and poll() from
continuing with stale availability data after real pointer errors.

Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
Cássio Gabriel (2):
      ALSA: compress: Use EOPNOTSUPP for missing pointer callbacks
      ALSA: compress: Propagate real pointer errors in avail paths

 sound/core/compress_offload.c | 55 ++++++++++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 17 deletions(-)
---
base-commit: 05b02f0ccc12a85c6b65cf9cc543151b160c7f49
change-id: 20260414-alsa-compress-pointer-avail-errors-d0b195cbac2d

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>

[PATCH 1/2] ALSA: compress: Use EOPNOTSUPP for missing pointer callbacks

[Cássio Gabriel]

snd_compr_update_tstamp() returns a sentinel error when a
compress driver does not implement the pointer() callback.

Use -EOPNOTSUPP for that case instead of -ENOTSUPP. This
keeps the behavior the same while switching to the standard
unsupported-operation errno and avoids the checkpatch warning
about ENOTSUPP not being a SUSv4 error code.

Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/core/compress_offload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index fd63d219bf86..e3239c739d34 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -186,7 +186,7 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 	int ret;
 
 	if (!stream->ops->pointer)
-		return -ENOTSUPP;
+		return -EOPNOTSUPP;
 
 	switch (stream->runtime->state) {
 	case SNDRV_PCM_STATE_OPEN:

-- 
2.53.0

[PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Cássio Gabriel]

Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
error out retrieving pointers") made snd_compr_update_tstamp()
return driver pointer() failures, but snd_compr_calc_avail()
still ignores that status and continues with stale runtime
counters.

The fallback for an unsupported pointer callback remains
intentional, so keep ignoring -EOPNOTSUPP there. Propagate
real pointer-refresh failures instead.

This makes SNDRV_COMPRESS_AVAIL, read() and write() fail
immediately on real pointer errors, and makes poll() report
EPOLLERR instead of continuing with stale availability data.
snd_compr_tstamp() already propagates the same errors, so
this also restores consistency between the timestamp and
avail paths.

Fixes: 61327f3d817c ("ALSA: compress: Pay attention if drivers error out retrieving pointers")
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/core/compress_offload.c | 53 ++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index e3239c739d34..5a125329d5b7 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -207,18 +207,23 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 	return 0;
 }
 
-static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
-				   struct snd_compr_avail64 *avail)
+static int snd_compr_calc_avail(struct snd_compr_stream *stream,
+				struct snd_compr_avail64 *avail)
 {
+	int ret;
+
 	memset(avail, 0, sizeof(*avail));
-	snd_compr_update_tstamp(stream, &avail->tstamp);
-	/* Still need to return avail even if tstamp can't be filled in */
+	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
+	if (ret < 0 && ret != -EOPNOTSUPP)
+		return ret;
+	/* Still need to return avail when no timestamp is available */
 
 	if (stream->runtime->total_bytes_available == 0 &&
 			stream->runtime->state == SNDRV_PCM_STATE_SETUP &&
 			stream->direction == SND_COMPRESS_PLAYBACK) {
 		pr_debug("detected init and someone forgot to do a write\n");
-		return stream->runtime->buffer_size;
+		avail->avail = stream->runtime->buffer_size;
+		return 0;
 	}
 	pr_debug("app wrote %llu, DSP consumed %llu\n",
 		 stream->runtime->total_bytes_available,
@@ -227,11 +232,12 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 				stream->runtime->total_bytes_transferred) {
 		if (stream->direction == SND_COMPRESS_PLAYBACK) {
 			pr_debug("both pointers are same, returning full avail\n");
-			return stream->runtime->buffer_size;
+			avail->avail = stream->runtime->buffer_size;
 		} else {
 			pr_debug("both pointers are same, returning no avail\n");
-			return 0;
+			avail->avail = 0;
 		}
+		return 0;
 	}
 
 	avail->avail = stream->runtime->total_bytes_available -
@@ -240,14 +246,21 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 		avail->avail = stream->runtime->buffer_size - avail->avail;
 
 	pr_debug("ret avail as %zu\n", (size_t)avail->avail);
-	return avail->avail;
+	return 0;
 }
 
-static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
+static inline int snd_compr_get_avail(struct snd_compr_stream *stream,
+				      size_t *avail_ret)
 {
 	struct snd_compr_avail64 avail;
+	int ret;
 
-	return snd_compr_calc_avail(stream, &avail);
+	ret = snd_compr_calc_avail(stream, &avail);
+	if (ret < 0)
+		return ret;
+
+	*avail_ret = avail.avail;
+	return 0;
 }
 
 static void snd_compr_avail32_from_64(struct snd_compr_avail *avail32,
@@ -262,15 +275,16 @@ static int snd_compr_ioctl_avail(struct snd_compr_stream *stream,
 {
 	struct snd_compr_avail64 ioctl_avail64;
 	struct snd_compr_avail ioctl_avail32;
-	size_t avail;
 	const void *copy_from = &ioctl_avail64;
 	size_t copy_size = sizeof(ioctl_avail64);
+	int ret;
 
 	if (stream->direction == SND_COMPRESS_ACCEL)
 		return -EBADFD;
 
-	avail = snd_compr_calc_avail(stream, &ioctl_avail64);
-	ioctl_avail64.avail = avail;
+	ret = snd_compr_calc_avail(stream, &ioctl_avail64);
+	if (ret < 0)
+		return ret;
 	if (is_32bit) {
 		snd_compr_avail32_from_64(&ioctl_avail32, &ioctl_avail64);
 		copy_from = &ioctl_avail32;
@@ -346,7 +360,9 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
 		return -EBADFD;
 	}
 
-	avail = snd_compr_get_avail(stream);
+	retval = snd_compr_get_avail(stream, &avail);
+	if (retval < 0)
+		return retval;
 	pr_debug("avail returned %lu\n", (unsigned long)avail);
 	/* calculate how much we can write to buffer */
 	if (avail > count)
@@ -402,7 +418,9 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
 		return -EPIPE;
 	}
 
-	avail = snd_compr_get_avail(stream);
+	retval = snd_compr_get_avail(stream, &avail);
+	if (retval < 0)
+		return retval;
 	pr_debug("avail returned %lu\n", (unsigned long)avail);
 	/* calculate how much we can read from buffer */
 	if (avail > count)
@@ -437,6 +455,7 @@ static __poll_t snd_compr_poll(struct file *f, poll_table *wait)
 	struct snd_compr_stream *stream;
 	struct snd_compr_runtime *runtime;
 	size_t avail;
+	int ret;
 	__poll_t retval = 0;
 
 	if (snd_BUG_ON(!data))
@@ -471,7 +490,9 @@ static __poll_t snd_compr_poll(struct file *f, poll_table *wait)
 	}
 #endif
 
-	avail = snd_compr_get_avail(stream);
+	ret = snd_compr_get_avail(stream, &avail);
+	if (ret < 0)
+		return snd_compr_get_poll(stream) | EPOLLERR;
 	pr_debug("avail is %lu\n", (unsigned long)avail);
 	/* check if we have at least one fragment to fill */
 	switch (runtime->state) {

-- 
2.53.0

Re: [PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On Tue, Apr 14, 2026 at 01:47:54PM -0300, Cássio Gabriel wrote:

> Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
> error out retrieving pointers") made snd_compr_update_tstamp()
> return driver pointer() failures, but snd_compr_calc_avail()
> still ignores that status and continues with stale runtime
> counters.

> The fallback for an unsupported pointer callback remains
> intentional, so keep ignoring -EOPNOTSUPP there. Propagate
> real pointer-refresh failures instead.

> -static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
> -				   struct snd_compr_avail64 *avail)
> +static int snd_compr_calc_avail(struct snd_compr_stream *stream,
> +				struct snd_compr_avail64 *avail)
>  {
> +	int ret;
> +
>  	memset(avail, 0, sizeof(*avail));
> -	snd_compr_update_tstamp(stream, &avail->tstamp);
> -	/* Still need to return avail even if tstamp can't be filled in */
> +	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
> +	if (ret < 0 && ret != -EOPNOTSUPP)
> +		return ret;
> +	/* Still need to return avail when no timestamp is available */

It's not clear to me that the intent there is to only ignore in the case
where we fail to update due to the operation not being supported rather
than to also ignore any other runtime errors we might encounter (eg, due
to the stream configuration).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Cássio Gabriel Monteiro Pires]

[-- Attachment #1.1: Type: text/plain, Size: 2619 bytes --]

On 4/14/26 13:53, Mark Brown wrote:
> On Tue, Apr 14, 2026 at 01:47:54PM -0300, Cássio Gabriel wrote:
> 
>> Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
>> error out retrieving pointers") made snd_compr_update_tstamp()
>> return driver pointer() failures, but snd_compr_calc_avail()
>> still ignores that status and continues with stale runtime
>> counters.
> 
>> The fallback for an unsupported pointer callback remains
>> intentional, so keep ignoring -EOPNOTSUPP there. Propagate
>> real pointer-refresh failures instead.
> 
>> -static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
>> -				   struct snd_compr_avail64 *avail)
>> +static int snd_compr_calc_avail(struct snd_compr_stream *stream,
>> +				struct snd_compr_avail64 *avail)
>>  {
>> +	int ret;
>> +
>>  	memset(avail, 0, sizeof(*avail));
>> -	snd_compr_update_tstamp(stream, &avail->tstamp);
>> -	/* Still need to return avail even if tstamp can't be filled in */
>> +	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
>> +	if (ret < 0 && ret != -EOPNOTSUPP)
>> +		return ret;
>> +	/* Still need to return avail when no timestamp is available */
> 
> It's not clear to me that the intent there is to only ignore in the case
> where we fail to update due to the operation not being supported rather
> than to also ignore any other runtime errors we might encounter (eg, due
> to the stream configuration).

Cool, thanks for replying!

I rechecked the current tree, and the hard-failure class does exist:
wm_adsp_compr_pointer() can return -EIO and switch the stream to
XRUN when dsp->fatal_error, !buf, or buf->error, so that is a
real in-tree case where pointer() failure means the stream itself is
broken rather than timestamp data simply being unavailable.

That said, I agree this does not fully justify the generic change as I
sent it. In particular, SNDRV_COMPRESS_AVAIL and poll() both recheck
the stream state after the pointer refresh, and there are other drivers
such as SOF where pointer() can return -EBUSY for a not-ready case,
which is not the same class as the wm_adsp -EIO/XRUN path.

I also agree that the current patch is too broad because it uses the raw pointer()
errno as the policy boundary for avail handling, and that conflates hard
stream-failure cases such as wm_adsp's -EIO/XRUN transition with
other conditions like SOF's -EBUSY not-ready return. I can rework patch
2 so the behavior is based on a better-defined failure condition instead
of treating every non-EOPNOTSUPP return as fatal for avail.

WDYT?

-- 
Thanks,
Cássio


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Takashi Iwai]

On Tue, 14 Apr 2026 19:45:52 +0200,
Cássio Gabriel Monteiro Pires wrote:
> 
> On 4/14/26 13:53, Mark Brown wrote:
> > On Tue, Apr 14, 2026 at 01:47:54PM -0300, Cássio Gabriel wrote:
> > 
> >> Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
> >> error out retrieving pointers") made snd_compr_update_tstamp()
> >> return driver pointer() failures, but snd_compr_calc_avail()
> >> still ignores that status and continues with stale runtime
> >> counters.
> > 
> >> The fallback for an unsupported pointer callback remains
> >> intentional, so keep ignoring -EOPNOTSUPP there. Propagate
> >> real pointer-refresh failures instead.
> > 
> >> -static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
> >> -				   struct snd_compr_avail64 *avail)
> >> +static int snd_compr_calc_avail(struct snd_compr_stream *stream,
> >> +				struct snd_compr_avail64 *avail)
> >>  {
> >> +	int ret;
> >> +
> >>  	memset(avail, 0, sizeof(*avail));
> >> -	snd_compr_update_tstamp(stream, &avail->tstamp);
> >> -	/* Still need to return avail even if tstamp can't be filled in */
> >> +	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
> >> +	if (ret < 0 && ret != -EOPNOTSUPP)
> >> +		return ret;
> >> +	/* Still need to return avail when no timestamp is available */
> > 
> > It's not clear to me that the intent there is to only ignore in the case
> > where we fail to update due to the operation not being supported rather
> > than to also ignore any other runtime errors we might encounter (eg, due
> > to the stream configuration).
> 
> Cool, thanks for replying!
> 
> I rechecked the current tree, and the hard-failure class does exist:
> wm_adsp_compr_pointer() can return -EIO and switch the stream to
> XRUN when dsp->fatal_error, !buf, or buf->error, so that is a
> real in-tree case where pointer() failure means the stream itself is
> broken rather than timestamp data simply being unavailable.
> 
> That said, I agree this does not fully justify the generic change as I
> sent it. In particular, SNDRV_COMPRESS_AVAIL and poll() both recheck
> the stream state after the pointer refresh, and there are other drivers
> such as SOF where pointer() can return -EBUSY for a not-ready case,
> which is not the same class as the wm_adsp -EIO/XRUN path.
> 
> I also agree that the current patch is too broad because it uses the raw pointer()
> errno as the policy boundary for avail handling, and that conflates hard
> stream-failure cases such as wm_adsp's -EIO/XRUN transition with
> other conditions like SOF's -EBUSY not-ready return. I can rework patch
> 2 so the behavior is based on a better-defined failure condition instead
> of treating every non-EOPNOTSUPP return as fatal for avail.
> 
> WDYT?

IMO, keeping the behavior could be safer for now.
The erroneous timestamp can be identified by zeros, at least.


thanks,

Takashi

Re: [PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Cássio Gabriel Monteiro Pires]

[-- Attachment #1.1: Type: text/plain, Size: 3046 bytes --]



On 4/15/26 11:20, Takashi Iwai wrote:
> On Tue, 14 Apr 2026 19:45:52 +0200,
> Cássio Gabriel Monteiro Pires wrote:
>>
>> On 4/14/26 13:53, Mark Brown wrote:
>>> On Tue, Apr 14, 2026 at 01:47:54PM -0300, Cássio Gabriel wrote:
>>>
>>>> Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
>>>> error out retrieving pointers") made snd_compr_update_tstamp()
>>>> return driver pointer() failures, but snd_compr_calc_avail()
>>>> still ignores that status and continues with stale runtime
>>>> counters.
>>>
>>>> The fallback for an unsupported pointer callback remains
>>>> intentional, so keep ignoring -EOPNOTSUPP there. Propagate
>>>> real pointer-refresh failures instead.
>>>
>>>> -static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
>>>> -				   struct snd_compr_avail64 *avail)
>>>> +static int snd_compr_calc_avail(struct snd_compr_stream *stream,
>>>> +				struct snd_compr_avail64 *avail)
>>>>  {
>>>> +	int ret;
>>>> +
>>>>  	memset(avail, 0, sizeof(*avail));
>>>> -	snd_compr_update_tstamp(stream, &avail->tstamp);
>>>> -	/* Still need to return avail even if tstamp can't be filled in */
>>>> +	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
>>>> +	if (ret < 0 && ret != -EOPNOTSUPP)
>>>> +		return ret;
>>>> +	/* Still need to return avail when no timestamp is available */
>>>
>>> It's not clear to me that the intent there is to only ignore in the case
>>> where we fail to update due to the operation not being supported rather
>>> than to also ignore any other runtime errors we might encounter (eg, due
>>> to the stream configuration).
>>
>> Cool, thanks for replying!
>>
>> I rechecked the current tree, and the hard-failure class does exist:
>> wm_adsp_compr_pointer() can return -EIO and switch the stream to
>> XRUN when dsp->fatal_error, !buf, or buf->error, so that is a
>> real in-tree case where pointer() failure means the stream itself is
>> broken rather than timestamp data simply being unavailable.
>>
>> That said, I agree this does not fully justify the generic change as I
>> sent it. In particular, SNDRV_COMPRESS_AVAIL and poll() both recheck
>> the stream state after the pointer refresh, and there are other drivers
>> such as SOF where pointer() can return -EBUSY for a not-ready case,
>> which is not the same class as the wm_adsp -EIO/XRUN path.
>>
>> I also agree that the current patch is too broad because it uses the raw pointer()
>> errno as the policy boundary for avail handling, and that conflates hard
>> stream-failure cases such as wm_adsp's -EIO/XRUN transition with
>> other conditions like SOF's -EBUSY not-ready return. I can rework patch
>> 2 so the behavior is based on a better-defined failure condition instead
>> of treating every non-EOPNOTSUPP return as fatal for avail.
>>
>> WDYT?
> 
> IMO, keeping the behavior could be safer for now.
> The erroneous timestamp can be identified by zeros, at least.

That sounds right, I'll drop this change for now.

-- 
Thanks,
Cássio


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]