sound: Fix chn_trigger() and vchan_trigger() races

Consider the following scenario:

1. CHN currently has its trigger set to PCMTRIG_STOP.
2. Thread A locks CHN, calls CHANNEL_TRIGGER(PCMTRIG_START)

sound: Fix chn_trigger() and vchan_trigger() races

Consider the following scenario:

1. CHN currently has its trigger set to PCMTRIG_STOP.
2. Thread A locks CHN, calls CHANNEL_TRIGGER(PCMTRIG_START), sets the
trigger to PCMTRIG_START and unlocks.
3. Thread B picks up the lock, calls CHANNEL_TRIGGER(PCMTRIG_ABORT) and
returns a non-zero value, so it returns from chn_trigger() as well.
4. Thread A picks up the lock and adds CHN to the list, which is
_wrong_, because the last call to CHANNEL_TRIGGER() was with
PCMTRIG_ABORT, meaning the channel is stopped, yet we are adding it
to the list and marking it as started.

Another problematic scenario:

1. Thread A locks CHN, sets the trigger to PCMTRIG_ABORT, and unlocks
CHN. It then locks PCM and _removes_ CHN from the list.
2. In the meantime, since thread A unlocked CHN, thread B has locked it,
set the trigger to PCMTRIG_START, unlocked it, and is now blocking on
PCM held by thread A.
3. At the same time, thread C locks CHN, sets the trigger back to
PCMTRIG_ABORT, unlocks CHN, and is also blocking on PCM. However,
once thread A unlocks PCM, because thread C is higher-priority than
thread B, it picks up the PCM lock instead of thread B, and because
CHN is already removed from the list, and thread B hasn't added it
back yet, we take a page fault in CHN_REMOVE() by trying to remove a
non-existent element.

To fix the former scenario, set the channel trigger before the call to
CHANNEL_TRIGGER() (could also come after, doesn't really matter) and
check if anything changed one we lock CHN back.

To fix the latter scenario, use the SAFE variants of CHN_INSERT_HEAD()
and CHN_REMOVE(). A similar scenario can occur in vchan_trigger(), so do
the trigger setting after we've locked the parent channel.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D47461

show more ...

sound: Remove redundant refcount checks in vchan_setnew()

When adding a new vchan, we are looking for a parent channel which
either already has vchans (i.e CHN_F_HAS_VCHAN), or does not, but is
also

sound: Remove redundant refcount checks in vchan_setnew()

When adding a new vchan, we are looking for a parent channel which
either already has vchans (i.e CHN_F_HAS_VCHAN), or does not, but is
also not being used (i.e !CHN_F_BUSY). Since CHN_F_BUSY essentially
tells us if the channel is currently being used or not, there is no need
to check if the channel's refcount is 0 as well.

When removing a vchan, we first check if we have only 1 vchan allocated
that is also being used (so we cannot remove it at the moment), and then
we check if the vchan is not busy and remove it. Again, checking
CHN_F_BUSY is enough.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D47268

show more ...

sound: Untangle dsp_cdevs[] and dsp_unit2name() confusion

Before de8c0d15a64fa ("sound: Get rid of snd_clone and use
DEVFS_CDEVPRIV(9)"), sound(4) would create one device for each allocated
channel.

sound: Untangle dsp_cdevs[] and dsp_unit2name() confusion

Before de8c0d15a64fa ("sound: Get rid of snd_clone and use
DEVFS_CDEVPRIV(9)"), sound(4) would create one device for each allocated
channel. The device names would be chosen from dsp_cdevs[], and created
with dsp_unit2name(). Additionally, dsp_cdevs[] was also used to match
these devices names, as well as OSSv4 aliases in dsp_clone().

Since sound(4) does not create separate devices for each channel
anymore, the meaning and use dsp_cdevs[] has changed. Part of it no
longer corresponds to devices at all, but instead is used to create
channel names, and another part is used to match only OSSv4 aliases in
dsp_clone().

To address this confusion, separate dsp_cdevs[] into a dsp_aliases[]
array, and move dsp_unit2name() to pcm/channel.c and rename it to
chn_mkname().

While here, get rid of the SND_DEV_DSPHW_* channel types, and simply use
the existing PCMDIR_* constants as the channel types. There is no need
to duplicate the same meaning twice.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D47199

show more ...

sound: Simplify channel creation and deletion process

Currently we create and destroy channels with the following consistent
pattern:

- chn_init() -> pcm_chn_add()
- pcm_chn_remove() -> chn_kill()

sound: Simplify channel creation and deletion process

Currently we create and destroy channels with the following consistent
pattern:

- chn_init() -> pcm_chn_add()
- pcm_chn_remove() -> chn_kill()

Instead of calling two separate functions, merge pcm_chn_add() with
chn_init(), and pcm_chn_remove() with chn_kill().

Another benefit of this change is that we avoid the confusion caused by
having pcm_chn_add(), as well as pcm_addchan().

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj
Differential Revision: https://reviews.freebsd.org/D46835

show more ...

sound: Remove useless newspd check in sysctl_dev_pcm_vchanrate()

feeder_rate_min functions as the lower boundary.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch

sound: Remove useless newspd check in sysctl_dev_pcm_vchanrate()

feeder_rate_min functions as the lower boundary.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46834

show more ...

sound: Simplify vchan_create() error paths

Instead of checking the value of "ret" multiple times, just set a goto
label and jump there immediately in case of an error.

While here, remove a redundan

sound: Simplify vchan_create() error paths

Instead of checking the value of "ret" multiple times, just set a goto
label and jump there immediately in case of an error.

While here, remove a redundant assignment to "d".

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D46833

show more ...

sound: Remove KASSERT from vchan_setnew()

This check is not really useful, and can in fact break things, if
sysctl_dev_pcm_vchans() calls vchan_setnew() with a value that will not
satisfy the KASSER

sound: Remove KASSERT from vchan_setnew()

This check is not really useful, and can in fact break things, if
sysctl_dev_pcm_vchans() calls vchan_setnew() with a value that will not
satisfy the KASSERT condition.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, emaste
Differential Revision: https://reviews.freebsd.org/D46545

show more ...

sound: Get rid of snd_sb16 workaround in vchan_create()

The snd_sb16 driver was removed in 716924cb4832 ("Retire snd_sbc ISA
sound card driver").

While here, simplify sample rate assignment a bit.

sound: Get rid of snd_sb16 workaround in vchan_create()

The snd_sb16 driver was removed in 716924cb4832 ("Retire snd_sbc ISA
sound card driver").

While here, simplify sample rate assignment a bit.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch, markj, emaste
Differential Revision: https://reviews.freebsd.org/D45662

show more ...

sound: Update pcm/vchan.c LICENSE header

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

sound: Remove unused "num" argument from chn_init() and related callers

It is always -1 (i.e unused).

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: dev_submerge.ch
Differentia

sound: Remove unused "num" argument from chn_init() and related callers

It is always -1 (i.e unused).

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D45095

show more ...

sound: Rename pcm_getparentchannel() to vchan_getparentchannel()

Follow the rest of the vchan.c naming convention.

No functional change intended.

Sponsored by: The FreeBSD Foundation
MFC after: 1

sound: Rename pcm_getparentchannel() to vchan_getparentchannel()

Follow the rest of the vchan.c naming convention.

No functional change intended.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D45016

show more ...

sound: Move vchan-related code to pcm/vchan.*

pcm/sound.* contains code that should be part of pcm/vchan.*.

Changes:
- pcm_setvchans() -> vchan_setnew()
- pcm_setmaxautovchans() -> vchan_setmaxauto

sound: Move vchan-related code to pcm/vchan.*

pcm/sound.* contains code that should be part of pcm/vchan.*.

Changes:
- pcm_setvchans() -> vchan_setnew()
- pcm_setmaxautovchans() -> vchan_setmaxauto()
- hw.snd.maxautovchans moved to pcm/vchan.c
- snd_maxautovchans declaration moved to pcm/vchan.h and definition to
pcm/vchan.c

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: dev_submerge.ch, markj
Differential Revision: https://reviews.freebsd.org/D45015

show more ...

sound: Convert pcm_chn_add() to void

It always returns 0.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: markj, emaste
Differential Revision: https://reviews.freebsd.org/D44998

sound: Merge pcm_chn_create() and chn_init()

Follow-up of b3ea087c05d8c75978a302cbb3fa92ce1afa3e49 ("sound: Merge
pcm_chn_destroy() and chn_kill()")

While here, add device_printf()'s to all failure

sound: Merge pcm_chn_create() and chn_init()

Follow-up of b3ea087c05d8c75978a302cbb3fa92ce1afa3e49 ("sound: Merge
pcm_chn_destroy() and chn_kill()")

While here, add device_printf()'s to all failure points. Also fix an
existing bug where we'd unlock an already unlocked channel, in case we
went to "out" (now "out2") before locking the channel.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D44993

show more ...

sound: Merge pcm_chn_destroy() and chn_kill()

pcm_chn_destroy() acts like a wrapper around chn_kill(), and
additionally calls a few more functions that should in fact be part of
chn_kill()'s logic.

sound: Merge pcm_chn_destroy() and chn_kill()

pcm_chn_destroy() acts like a wrapper around chn_kill(), and
additionally calls a few more functions that should in fact be part of
chn_kill()'s logic. Merge pcm_chn_destroy()'s functionality in
chn_kill() to improve readability, as well as code layering.

While here, convert chn_kill() to void as it currently always returns 0.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D44984

show more ...

sound: Retire unit.*

The unit.* code is largely obsolete and imposes limits that are no
longer needed nowadays.

- Capping the maximum allowed soundcards in a given machine. By default,
the limit

sound: Retire unit.*

The unit.* code is largely obsolete and imposes limits that are no
longer needed nowadays.

- Capping the maximum allowed soundcards in a given machine. By default,
the limit is 512 (snd_max_u() in unit.c), and the maximum possible is
2048 (SND_UNIT_UMAX in unit.h). It can also be tuned through the
hw.snd.maxunit loader(8) tunable. Even though these limits are large
enough that they should never cause problems, there is no need for
this limit to exist in the first place.
- Capping the available device/channel types. By default, this is 32
(snd_max_d() in unit.c). However, these types are pre-defined in
pcm/sound.h (see SND_DEV_*), so the cap is unnecessary when we know
that their number is constant.
- Capping the number of channels per-device. By default, the limit 1024
(snd_max_c() in unit.c). This is probably the most problematic of the
limits mentioned, because this limit can never be reached, as the
maximum is hard-capped at either hw.snd.maxautovchans (16 by default),
or SND_MAXHWCHAN and SND_MAXVCHANS.

These limtits are encoded in masks (see SND_U_MASK, SND_D_MASK,
SND_C_MASK in unit.h) and are used to construct a bitfield of the form
[dsp_unit, type, channel_unit] in snd_mkunit() which is assigned to
pcm_channel->unit.

This patch gets rid of everything unit.*-related and makes a slightly
different use of the "unit" field to only contain the channel unit
number. The channel type is stored in a new pcm_channel->type field, and
the DSP unit number need not be stored at all, since we can fetch it
from device_get_unit(pcm_channel->dev). This change has the effect that
we no longer need to impose caps on the number of soundcards,
device/channel types and per-device channels. As a result the code is
noticeably simplified and more readable.

Apart from the fact that the hw.snd.maxunit loader(8) tunable is also
retired as a side-effect of this patch, sound(4)'s behavior remains the
same.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D44912

show more ...

sys: Remove SND_DECLARE_FILE

Reviewed by: kbowling, imp, emaste
Differential Revision: https://reviews.freebsd.org/D41499

sys: Remove $FreeBSD$: sound driver version

Remove /SND_DECLARE_FILE\("\$FreeBSD\$"\);/

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.

Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix

show more ...

sound: clean up empty lines in .c and .h files

Merge ^/head r358269 through r358399.

Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (17 of many)

r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly mark

Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (17 of many)

r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.

This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.

Mark all obvious cases as MPSAFE. All entries that haven't been marked
as MPSAFE before are by default marked as NEEDGIANT

Approved by: kib (mentor, blanket)
Commented by: kib, gallatin, melifaro
Differential Revision: https://reviews.freebsd.org/D23718

show more ...

sys/dev: further adoption of SPDX licensing ID tags.

Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error

sys/dev: further adoption of SPDX licensing ID tags.

Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

show more ...

Catch up with HEAD (r280229-r284686).

Merge sync of head