On Fri, 11 Dec 2020 18:20:50 +0100, Jaroslav Kysela wrote:
Dne 11. 12. 20 v 17:59 Takashi Iwai napsal(a):
On Fri, 11 Dec 2020 17:45:45 +0100, Jaroslav Kysela wrote:
Dne 11. 12. 20 v 9:38 Takashi Iwai napsal(a):
Currently alsactl-restore tries to initialize the device when an error is found for restore action. But this isn't the right behavior in the case where the lock is held; it implies that another alsactl is running concurrently, hence you shouldn't initialize the card at the same time. The situation is found easily when two alsactls get started by both udev and systemd (note that those two invocations are the designed behavior, see /usr/lib/udev/rules.d/78-sound-cards.rules for details).
This patch changes load_state() not to handle the initialization if the locking fails.
The operation should serialize in this case (there's limit of 10 seconds which should be enough to finish the initialization). The state_lock() function should return -EBUSY when the file is locked (and I'm fine to change the behaviour from 'init' to 'skip' for this lock state).
It seems that -EEXIST is returned when the lock file exists, but the open(file, O_CREAT|O_EXCL, 0644) caller has not enough priviledges to access this file when another user owns the file.
But alsactl from /lib/udev/rules.d/90-alsa-restore.rules and /lib/systemd/system/alsa-restore.service should be run as root, right?
Yes, it should be root.
I also wondered how EEXIST comes, too. Maybe it's also the race between the first open(O_RDWR) and the second open(O_RDWR|O_CREAT|O_EXCL)? If so, it'd be better to go back again to the normal open(O_RDWR)?
Maybe. It seems enough to add EEXIST errno check to the "if (errno == EBUSY || errno == EAGAIN)" condition to repeat the open sequence. The -EBUSY will be returned correctly then. The one second delay is harmless in my eyes for the second task.
I'm afraid that a significant delay can be confusing. And this should be the race only once, so no need to add the artificial delay, IMO.
(BTW, now I noticed that we decrease timeout twice :)
Takashi