sound: Simplify pcm_chnalloc() and fix infinite loop bug

[christos]

Summary

Simplify logic to execute the following algorithm:

1. Search for a free (i.e not busy) channel and return successfully.
2. If no channel was found, either return an ENOTSUP if no VCHAN can be created (disabled or we reached the limit), or create one more VCHAN and scan for it again.
3. If the second scan failed again, return EBUSY.

This patch also solves a bug where we'd end up in an infinite loop,
calling vchan_setnew() with the same newcnt value indefinitely, after
the following scenario:

1. Plug device.
2. Spawn X channels.
3. Kill all channels except _last_ opened.
4. Clean up all unused VCHANs.
5. Spawn X+1 channels.
6. Infinite loop in pcm_chnalloc().

I am not exactly sure which part of pcm_chnalloc() caused this bug, but
the patch fixes it at least...

Sponsored by: The FreeBSD Foundation
MFC after: 2 days

[christos]

Handle vchan_setnew() error case.

[dev_submerge.ch]

I think the endless loop was caused by goto vchan_alloc;: It skips the retry check, successfully calls vchan_setnew() with unchanged values, and then loops back through goto retry_chnalloc;. There's nothing else in there that would change and break the loop.

That endless loop would be triggered when the vchan_num < c->unit condition is met. I'd expect that to happen before spawning vchan X + 1 though.

[christos]

In D46548#1063912, @dev_submerge.ch wrote:

I think the endless loop was caused by goto vchan_alloc;: It skips the retry check, successfully calls vchan_setnew() with unchanged values, and then loops back through goto retry_chnalloc;. There's nothing else in there that would change and break the loop.

That endless loop would be triggered when the vchan_num < c->unit condition is met. I'd expect that to happen before spawning vchan X + 1 though.

Good analysis. I did some testing to confirm this a few days ago and I think this is what's happening indeed. Are you okay with the patch?