sound: Retire PCM_GIANT

[christos]

Summary

We already lock the softc using PCM_[UN]LOCK() so there is no reason to
use Giant anymore.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days

[emaste]

sys/dev/sound/pcm/mixer.c

1566

can we KASSERT m->lock is not NULL?

[christos]

sys/dev/sound/pcm/mixer.c

1566

Since we create these locks with snd_mtxcreate(), would it better to check if mtx_init() succeeded instead of checking here every time?

[markj]

We already lock the softc using PCM_[UN]LOCK() so there is no reason to use Giant anymore.

There is other state that might be synchronized by Giant: global variables, the list of pcm drivers. For instance, code which references pcm_devclass should be locked with bus_topo_lock() (which is just Giant under the hood).

sys/dev/sound/pcm/sound.h

432

Did you verify that all drivers set SD_F_MPSAFE? We should assert somewhere, with an assertion, that all of them do.

[imp]

In D46698#1064652, @markj wrote:

We already lock the softc using PCM_[UN]LOCK() so there is no reason to use Giant anymore.

There is other state that might be synchronized by Giant: global variables, the list of pcm drivers. For instance, code which references pcm_devclass should be locked with bus_topo_lock() (which is just Giant under the hood).

Yes. Since you may have other threads that attach / detach devices, you need the bus_topo_lock() ... bus_topo_unlock() around this to prevent races with devices detaching while it's running.

topo lock will likely stop being Giant soonish, so it's good to the wrappers. There's no need to mark things using it as Giant, since that's for automatic locking and we're explicitly locking things here.

[christos]

In D46698#1064652, @markj wrote:

We already lock the softc using PCM_[UN]LOCK() so there is no reason to use Giant anymore.

There is other state that might be synchronized by Giant: global variables, the list of pcm drivers. For instance, code which references pcm_devclass should be locked with bus_topo_lock() (which is just Giant under the hood).

pcm_devclass wasn't Giant locked anywhere already, except in the clone routines. Wouldn't it be better to do it in a separate patch?

sys/dev/sound/pcm/sound.h

432

Only some drivers set SD_F_MPSAFE (including snd_uaudio and snd_hda). Why do we need to check this though?

[christos]

Abandoning after in-person discussion with Mark.