On Fri, 11 Dec 2020 17:59:05 +0100,
Takashi Iwai wrote:
>
> 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)?
... something like below
diff --git a/alsactl/lock.c b/alsactl/lock.c
index 4a485392b3bd..c1c30f0c5eee 100644
--- a/alsactl/lock.c
+++ b/alsactl/lock.c
@@ -64,6 +64,9 @@ static int state_lock_(const char *file, int lock, int timeout, int _fd)
if (errno == EBUSY || errno == EAGAIN) {
sleep(1);
timeout--;
+ } if (errno == EEXIST){
+ /* race at creating a lock, try again */
+ continue;
} else {
err = -errno;
goto out;
Takashi
