Page MenuHomeFreeBSD

sound: Get rid of pnum and max variables in chn_init()
ClosedPublic

Authored by christos on Sep 3 2024, 3:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 8 2024, 12:51 AM
Unknown Object (File)
Dec 8 2024, 12:50 AM
Unknown Object (File)
Dec 8 2024, 12:50 AM
Unknown Object (File)
Dec 6 2024, 4:51 AM
Unknown Object (File)
Dec 5 2024, 8:17 AM
Unknown Object (File)
Nov 23 2024, 3:57 PM
Unknown Object (File)
Nov 22 2024, 2:39 PM
Unknown Object (File)
Nov 18 2024, 7:09 PM
Subscribers

Details

Summary

The VCHAN count is checked in vchan_setnew(), and there is no reason to
cap the hardware channels in the first place.

This is part of a series of follow-up patches.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/dev/sound/pcm/channel.c
1205–1206

Not related to this patch, but to the overall topic of vchan_setnew(): I suppose the unit number should be unique, right? Are we sure this is the case when somebody lowers the vchan sysctls and vchan_setnew() garbage collects some of the intermediate vchans?

sys/dev/sound/pcm/channel.c
1205–1206

If I understand correctly, what you are asking is the following. Suppose we have the following state:

vp0 -> unused
vp1 -> unused
vp2 -> someapp

If we lower the VCHAN amount to 1, we'll get the following:

vp2 -> someapp

Then, we set them to 4, and spawn 3 VCHANs:

vp0 -> foo
vp1 -> bar
vp2 -> someapp?
vp3 -> baz?

Apparently, after some testing now, I found out that in this scenario we'll override vp2 and essentially break things. Currently chn_init() does not make sure the unit doesn't already exist and instead just picks the length of the channel list (for a given direction) as the new unit number, which, as demonstrated in this example, can result in picking a unit number that already exists, when creating the 3rd channel.

What we should be doing in this loop is checking that c->unit != unit.

sys/dev/sound/pcm/channel.c
1205–1206

Bingo! ;-)
Didn't have time to test it yesterday before I had to leave.

Any comment related to the patch?

sys/dev/sound/pcm/channel.c
1205–1206

I will fix this ASAP, as it currently hangs sound(4). I think we end up in some infinite loop in either vchan_setnew() or something. Will have to investigate...

christos added inline comments.
sys/dev/sound/pcm/channel.c
1205–1206

@dev_submerge.ch D46548 fixes the actual hang (although, as mentioned in the commit message) I am not completely sure which part exactly caused it, and D46550 fixes the unique unit problem. D46549 is needed in order to simplify the unit assignment (easier to work with a sorted list).

This revision is now accepted and ready to land.Sep 13 2024, 6:33 PM