fcntl spinlock in Linux?

[Jeff Layton]

On Thu, 14 Feb 2013 20:12:59 +1030
Rusty Russell <rusty at samba.org> wrote:

> Volker Lendecke <Volker.Lendecke at SerNet.DE> writes:
> > Also, I am pretty certain that the fact that this happens in
> > an otherwise overloaded system adds to that effect, during
> > my tests here the systems were otherwise idle.
> 
> Oops, I dropped the list from my CC's somehow.
> 
> OK, I'll assume that improving this benchmark with 10000 or 20000 mean
> we have a real improvement.
> 
> I haven't tested, but one thing I noticed looking at the code: we're
> often deleted from the block list when we're woken, so we don't need
> to re-grab the lock in that case:
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index a94e331..41de9eb 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -498,6 +498,9 @@ static void __locks_delete_block(struct file_lock *waiter)
>   */
>  void locks_delete_block(struct file_lock *waiter)
>  {
> +	/* We could already be deleted by the time this runs. */
> +	if (list_empty(&waiter->fl_block))
> +		return;
>  	lock_flocks();
>  	__locks_delete_block(waiter);
>  	unlock_flocks();
> 
> Cheers,
> Rusty.
> 


Rusty,

I'm working some more on this problem. Would you have any issue with me
modifying the testcase you have below and adding it to the testsuite I
have for testing these patches?



> // 2>/dev/null; set -e; OUT=/tmp/`basename $0 .c`; if [ ! -f "$OUT" ] || [ "$OUT" -ot "$0" ]; then gcc -Wall -g -o "$OUT".$$ $0 -lrt && mv "$OUT".$$ "$OUT"; fi; exec "$OUT" "$@"
> #define HAVE_CLOCK_GETTIME_IN_LIBRT 1
> #define HAVE_STRUCT_TIMESPEC 1
> #include <err.h>
> #include <stdlib.h>
> #include <string.h>
> #include <stdio.h>
> #include <sys/types.h>
> #include <regex.h>
> #include <assert.h>
> #include <ctype.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <stdbool.h>
> 
> /* Taken from ccan/time: Licensed under BSD-MIT - see LICENSE file for details */
> #if HAVE_STRUCT_TIMESPEC
> #include <time.h>
> #else
> struct timespec {
> 	time_t   tv_sec;        /* seconds */
> 	long     tv_nsec;       /* nanoseconds */
> };
> #endif
> 
> #if !HAVE_CLOCK_GETTIME && !HAVE_CLOCK_GETTIME_IN_LIBRT
> #include <sys/time.h>
> 
> struct timespec time_now(void)
> {
> 	struct timeval now;
> 	struct timespec ret;
> 	gettimeofday(&now, NULL);
> 	ret.tv_sec = now.tv_sec;
> 	ret.tv_nsec = now.tv_usec * 1000;
> 	return ret;
> }
> #else
> #include <time.h>
> struct timespec time_now(void)
> {
> 	struct timespec ret;
> 	clock_gettime(CLOCK_REALTIME, &ret);
> 	return ret;
> }
> #endif /* HAVE_CLOCK_GETTIME || HAVE_CLOCK_GETTIME_IN_LIBRT */
> 
> static struct timespec time_sub(struct timespec recent,
> 				       struct timespec old)
> {
> 	struct timespec diff;
> 
> 	diff.tv_sec = recent.tv_sec - old.tv_sec;
> 	if (old.tv_nsec > recent.tv_nsec) {
> 		diff.tv_sec--;
> 		diff.tv_nsec = 1000000000 + recent.tv_nsec - old.tv_nsec;
> 	} else
> 		diff.tv_nsec = recent.tv_nsec - old.tv_nsec;
> 
> 	return diff;
> }
> 
> /* Taken from TDB: LGPLv3 */
> static int fcntl_lock(int fd, int rw, off_t off, off_t len, bool waitflag)
> {
> 	struct flock fl;
> 
> 	fl.l_type = rw;
> 	fl.l_whence = SEEK_SET;
> 	fl.l_start = off;
> 	fl.l_len = len;
> 	fl.l_pid = 0;
> 
> 	if (waitflag)
> 		return fcntl(fd, F_SETLKW, &fl);
> 	else
> 		return fcntl(fd, F_SETLK, &fl);
> }
> 
> static int fcntl_unlock(int fd, int rw, off_t off, off_t len)
> {
> 	struct flock fl;
> 	fl.l_type = F_UNLCK;
> 	fl.l_whence = SEEK_SET;
> 	fl.l_start = off;
> 	fl.l_len = len;
> 	fl.l_pid = 0;
> 
> 	return fcntl(fd, F_SETLKW, &fl);
> }
> 
> int main(int argc, char *argv[])
> {
> 	int num = 5000, i;
> 	const char *lockfile = argv[1];
> 	char *fill;
> 	int lockfd, to_lockers[2], from_lockers[2];
> 	struct timespec start, diff;
> 
> 	if (argc > 2) {
> 		num = atoi(argv[2]);
> 		argv++;
> 		argc--;
> 	}
> 	if (argc != 2)
> 		errx(1, "Usage: lock-stampede <filename> [num]");
> 
> 	pipe(to_lockers);
> 	pipe(from_lockers);
> 	lockfd = open(lockfile, O_RDWR);
> 	if (lockfd < 0)
> 		err(1, "Opening %s", lockfile);
> 
> 	start = time_now();
> 	for (i = 0; i < num; i++) {
> 		char c;
> 
> 		switch (fork()) {
> 		case 0:
> 			close(from_lockers[0]);
> 			close(to_lockers[1]);
> 			/* Parent dies? */
> 			if (read(to_lockers[0], &c, 1) != 1)
> 				exit(0);
> 			if (fcntl_lock(lockfd, F_WRLCK, i, 1, true) != 0)
> 				err(1, "Locking %s", lockfile);
> 			/* Tell parent we got it! */
> 			if (write(from_lockers[1], &c, 1) != 1)
> 				err(1, "Writing to parent");
> 			/* If we exit, we'd release it, so wait. */
> 			read(to_lockers[0], &c, 1);
> 			exit(0);
> 		case -1:
> 			err(1, "fork failed");
> 		}
> 	}
> 
> 	close(to_lockers[0]);
> 	close(from_lockers[1]);
> 
> 	diff = time_sub(time_now(), start);
> 	printf("Children set up in %lu.%09u\n",
> 	       (long)diff.tv_sec, (int)diff.tv_nsec);
> 
> 	if (fcntl_lock(lockfd, F_WRLCK, 0, num, true) != 0)
> 		err(1, "Locking %u bytes in %s", num, lockfile);
> 
> 	fill = calloc(num, 1);
> 
> 	/* OK, now wake all the kids: they block on lock. */
> 	i = 0;
> 	do {
> 		int ret = write(to_lockers[1], fill + i, num - i);
> 
> 		if (ret < 0)
> 			err(1, "writing to wake up locker children");
> 		i += ret;
> 	} while (i < num);
> 
> 	sleep(1);
> 
> 	start = time_now();
> 	fcntl_unlock(lockfd, F_WRLCK, 0, num);
> 	for (i = 0; i < num; i++)
> 		if (read(from_lockers[0], fill, 1) != 1)
> 			err(1, "Reading from locker children");
> 	diff = time_sub(time_now(), start);
> 	printf("Children got locks in %lu.%09u\n",
> 	       (long)diff.tv_sec, (int)diff.tv_nsec);
> 
> 	/* Children will now exit. */
> 	exit(0);
> }


-- 
Jeff Layton <jlayton at samba.org>