Revision 1.12 of lockf.c fixed a "thundering herd" scenario when the
lock experienced contention a number of processes would race to acquire lock when it was released. This problem resulted in a lot of CPU load as well as locks being picked up out of order. Unfortunately, a regression snuck in which allowed multiple threads to pickup the same lock when -k was not used. This could occur when multiple processes open a file descriptor to inode X (one process will be blocked) and the file is unlinked on unlock (thereby removing the directory entry allow another process to create a new directory entry for the same file name and lock it). This changes restores the old algorithm of: wait for the lock, then acquire lock when we want to unlink the file on exit (specifically when -k is not used) and keeps the new algorithm for when -k is used, which yields fairness and improved performance. Also, update the man page to inform users that if lockf(1) is being used to facilitate concurrency between a number of processes, it is recommended that -k be used to reduce CPU load and yeld fairness with regard to lock ordering. Collaborated with: jdp PR: bin/114341 PR: bin/116543 PR: bin/111101 MFC after: 1 week
This commit is contained in:
parent
4c5ada1230
commit
6a53f0a52b
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=172580
@ -1,4 +1,4 @@
|
||||
.\"
|
||||
\"
|
||||
.\" Copyright (C) 1998 John D. Polstra. All rights reserved.
|
||||
.\"
|
||||
.\" Redistribution and use in source and binary forms, with or without
|
||||
@ -66,6 +66,18 @@ the mere existence of the
|
||||
.Ar file
|
||||
is not considered to constitute a lock.
|
||||
.Pp
|
||||
If the
|
||||
.Nm
|
||||
utility is being used to facilitate concurrency between a number
|
||||
of processes, it is recommended that the
|
||||
.Fl k
|
||||
option be used. This will guarantee lock ordering, as well as implement
|
||||
a performance enhanced algorithm which minimizes CPU load associated
|
||||
with concurrent unlink, drop and re-acquire activity. It should be noted
|
||||
that if the
|
||||
.Fl k
|
||||
option is not used, then no guarantees around lock ordering can be made.
|
||||
.Pp
|
||||
The following options are supported:
|
||||
.Bl -tag -width ".Fl t Ar seconds"
|
||||
.It Fl k
|
||||
|
@ -38,11 +38,12 @@ __FBSDID("$FreeBSD$");
|
||||
#include <sysexits.h>
|
||||
#include <unistd.h>
|
||||
|
||||
static int acquire_lock(const char *name, int flags);
|
||||
static void cleanup(void);
|
||||
static void killed(int sig);
|
||||
static void timeout(int sig);
|
||||
static void usage(void);
|
||||
static int wait_for_lock(const char *name, int flags);
|
||||
static void wait_for_lock(const char *name);
|
||||
|
||||
static const char *lockname;
|
||||
static int lockfd = -1;
|
||||
@ -95,9 +96,37 @@ main(int argc, char **argv)
|
||||
sigaction(SIGALRM, &act, NULL);
|
||||
alarm(waitsec);
|
||||
}
|
||||
lockfd = wait_for_lock(lockname, O_NONBLOCK);
|
||||
while (lockfd == -1 && !timed_out && waitsec != 0)
|
||||
lockfd = wait_for_lock(lockname, 0);
|
||||
/*
|
||||
* If the "-k" option is not given, then we must not block when
|
||||
* acquiring the lock. If we did, then the lock holder would
|
||||
* unlink the file upon releasing the lock, and we would acquire
|
||||
* a lock on a file with no directory entry. Then another
|
||||
* process could come along and acquire the same lock. To avoid
|
||||
* this problem, we separate out the actions of waiting for the
|
||||
* lock to be available and of actually acquiring the lock.
|
||||
*
|
||||
* That approach produces behavior that is technically correct;
|
||||
* however, it causes some performance & ordering problems for
|
||||
* locks that have a lot of contention. First, it is unfair in
|
||||
* the sense that a released lock isn't necessarily granted to
|
||||
* the process that has been waiting the longest. A waiter may
|
||||
* be starved out indefinitely. Second, it creates a thundering
|
||||
* herd situation each time the lock is released.
|
||||
*
|
||||
* When the "-k" option is used, the unlink race no longer
|
||||
* exists. In that case we can block while acquiring the lock,
|
||||
* avoiding the separate step of waiting for the lock. This
|
||||
* yields fairness and improved performance.
|
||||
*/
|
||||
lockfd = acquire_lock(lockname, O_NONBLOCK);
|
||||
while (lockfd == -1 && !timed_out && waitsec != 0) {
|
||||
if (keep)
|
||||
lockfd = acquire_lock(lockname, 0);
|
||||
else {
|
||||
wait_for_lock(lockname);
|
||||
lockfd = acquire_lock(lockname, O_NONBLOCK);
|
||||
}
|
||||
}
|
||||
if (waitsec > 0)
|
||||
alarm(0);
|
||||
if (lockfd == -1) { /* We failed to acquire the lock. */
|
||||
@ -125,6 +154,25 @@ main(int argc, char **argv)
|
||||
return (WIFEXITED(status) ? WEXITSTATUS(status) : 1);
|
||||
}
|
||||
|
||||
/*
|
||||
* Try to acquire a lock on the given file, creating the file if
|
||||
* necessary. The flags argument is O_NONBLOCK or 0, depending on
|
||||
* whether we should wait for the lock. Returns an open file descriptor
|
||||
* on success, or -1 on failure.
|
||||
*/
|
||||
static int
|
||||
acquire_lock(const char *name, int flags)
|
||||
{
|
||||
int fd;
|
||||
|
||||
if ((fd = open(name, O_RDONLY|O_CREAT|O_EXLOCK|flags, 0666)) == -1) {
|
||||
if (errno == EAGAIN || errno == EINTR)
|
||||
return (-1);
|
||||
err(EX_CANTCREAT, "cannot open %s", name);
|
||||
}
|
||||
return (fd);
|
||||
}
|
||||
|
||||
/*
|
||||
* Remove the lock file.
|
||||
*/
|
||||
@ -173,16 +221,17 @@ usage(void)
|
||||
|
||||
/*
|
||||
* Wait until it might be possible to acquire a lock on the given file.
|
||||
* If the file does not exist, return immediately without creating it.
|
||||
*/
|
||||
static int
|
||||
wait_for_lock(const char *name, int flags)
|
||||
static void
|
||||
wait_for_lock(const char *name)
|
||||
{
|
||||
int fd;
|
||||
|
||||
if ((fd = open(name, O_CREAT|O_RDONLY|O_EXLOCK|flags, 0666)) == -1) {
|
||||
if (errno == EINTR || errno == EAGAIN)
|
||||
return (-1);
|
||||
if ((fd = open(name, O_RDONLY|O_EXLOCK, 0666)) == -1) {
|
||||
if (errno == ENOENT || errno == EINTR)
|
||||
return;
|
||||
err(EX_CANTCREAT, "cannot open %s", name);
|
||||
}
|
||||
return (fd);
|
||||
close(fd);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user