sound: Simplify locking in sysctl_dev_pcm_bitperfect()

[christos]

Summary

There is no need to lock and acquire SD_F_BUSY, when in reality we only
really need to lock when modifying d->flags.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

[christos]

PCM_ACQUIRE() sets the SD_F_BUSY flag, which more or less works as a sleepable lock. I do not really see the purpose of doing that here. Also I do not see the purpose of locking at the start of the function, just to read d->flags & SD_F_BITPERFECT.

[kib]

sys/dev/sound/pcm/sound.c

303

Why is d valid at this point?

[christos]

sys/dev/sound/pcm/sound.c

303

The PCM_REGISTERED(d) checks that. Do you mean how do we know it won't get destroyed in between PCM_REGISTERED() and here?

[christos]

sys/dev/sound/pcm/sound.c

303

If yes, then this is actually something I'm thinking about as well, but because this question can apply to every single place where we use d throughout sound(4). Because we support hot-unplugging, this means the device can disappear at any time, so the risk of the device going away is always present (see D53844 also). I'm wondering if that means that we basically have to lock wherever we use d. What do you think?

[kib]

sys/dev/sound/pcm/sound.c

303

Lock as in locking(9) might be too hard to use there, but yes, you need some concept of busy status, which would serialize between soft-destroy and running thread. See for instance how dev_refthread() handles cdevsw methods executing vs. destroy_dev().

Without some kind of such mechanism, I do not see why is it safe to even call PCM_REGISTERED().

[christos]

sys/dev/sound/pcm/sound.c

303

Yeah, I need to think a bit more about it actually. pcm_unregister() is responsible for destroying the device instance. It first clears the SD_F_REGISTERED flag, which is checked by all syscalls and a bunch of other functions through PCM_REGISTERED/DSP_REGISTERED. The problem is that in many cases we need to unlock for a while, and there is no guarantee that pcm_unregister() won't be triggered during that window (as I demonstrated it D53844). And I don't think checking for SD_F_REGISTERED every single time we acquire the lock makes sense either. But I'm not sure serializing every single call by locking d for the duration of the function call makes sense either, it'd make things pretty slower I imagine.

I'm trying to find a clean way to handle this. I'm trying to see how kern_conf.c does things, but if you have some idea feel free to let me know :-)

[christos]

sys/dev/sound/pcm/sound.c

303

I'm abandoning this and some other related ones. Feel free to reply in an email to the comment above. Thanks.