[LTP] [PATCH] syscalls/fcntl36: add tests for OFD locks

[LTP] [PATCH] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 315 ++++++++++++++++++++++++++++++
 2 files changed, 318 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..4c55279
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,315 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_locks";
+static int write_flag_cnt;
+const int write_size = 4096;
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+static void spawn_threads(pthread_t *id, void *(*thread_fn)(void *))
+{
+	intptr_t i;
+
+	for (i = 0; i < thread_cnt; ++i)
+		SAFE_PTHREAD_CREATE(id + i, NULL, thread_fn, (void *)i);
+}
+
+static void wait_threads(pthread_t *id)
+{
+	int i;
+
+	for (i = 0; i < thread_cnt; ++i)
+		SAFE_PTHREAD_JOIN(id[i], NULL);
+}
+
+/* Loop OFD wlock writing data*/
+static void *thread_fn_ofd_wlock(void *arg)
+{
+	unsigned char buf[write_size];
+	long start = (intptr_t)arg * write_size;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	int i = 0;
+
+	/* write with half writesize offset in the second spawning */
+	start += (write_flag_cnt % 2) * write_size / 2;
+	memset(buf, 'o', write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = start,
+		.l_len    = write_size,
+		.l_pid    = 0,
+	};
+
+	struct timespec ts = {
+               .tv_sec = 0,
+               .tv_nsec = 50000000,
+	};
+
+	for (; i < thread_cnt; i++) {
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, start, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, write_size);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		nanosleep(&ts, NULL);
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* Loop POSIX wlock writing data*/
+static void *thread_fn_posix_wlock(void *arg)
+{
+	unsigned char buf[write_size];
+	long start = (intptr_t)arg * write_size;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	int i = 0;
+
+	memset(buf, 'p', write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = start,
+		.l_len    = write_size,
+	};
+
+	for (; i < thread_cnt; i++) {
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, start, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, write_size);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* Loop OFD rlock reading data*/
+static void *thread_fn_ofd_rlock(void *arg)
+{
+	unsigned char buf[write_size];
+	long start = (intptr_t)arg * write_size;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	int i = 0;
+
+	memset(buf, (intptr_t)arg, write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = start,
+		.l_len    = write_size,
+		.l_pid    = 0,
+	};
+
+	for (; i < thread_cnt; i++) {
+
+		lck.l_type = F_RDLCK;
+		if (fcntl(fd, F_OFD_SETLK, &lck) == -1) {
+			/* can only fail with EAGAIN */
+			if (errno != EAGAIN)
+				tst_brk(TBROK, "errno not equal to EAGAIN");
+		} else {
+
+			/* rlock acquired */
+			SAFE_LSEEK(fd, start, SEEK_SET);
+			SAFE_READ(0, fd, buf, write_size);
+
+			lck.l_type = F_UNLCK;
+			SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+		}
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* Loop POSIX rlock reading data */
+static void *thread_fn_posix_rlock(void *arg)
+{
+	unsigned char buf[write_size];
+	long start = (intptr_t)arg * write_size;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	int i = 0;
+
+	memset(buf, (intptr_t)arg, write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = start,
+		.l_len    = write_size,
+	};
+
+	for (; i < thread_cnt; i++) {
+
+		lck.l_type = F_RDLCK;
+		if (fcntl(fd, F_SETLK, &lck) == -1) {
+			/* can only fail with EAGAIN */
+			if (errno != EAGAIN)
+				tst_brk(TBROK, "errno not equal to EAGAIN");
+		} else {
+
+			/* rlock acquired */
+			SAFE_LSEEK(fd, start, SEEK_SET);
+			SAFE_READ(0, fd, buf, write_size);
+
+			lck.l_type = F_UNLCK;
+			SAFE_FCNTL(fd, F_SETLK, &lck);
+		}
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+static int test_fn(void *f0(void *), void *f1(void *))
+{
+	intptr_t i;
+	int k;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	unsigned char buf[write_size];
+
+	int fd = SAFE_OPEN(fname, O_CREAT | O_TRUNC | O_RDWR, 0600);
+
+	spawn_threads(id0, f0);
+	write_flag_cnt++;
+	spawn_threads(id1, f1);
+	wait_threads(id0);
+	wait_threads(id1);
+
+	tst_res(TINFO, "Verifying file's data");
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+
+	for (i = 0; i <= thread_cnt; ++i) {
+
+		SAFE_READ(1, fd, buf, write_size/2);
+
+		if (buf[0] != 'o' && buf[0] != 'p') {
+			tst_res(TFAIL, "unexpected data read");
+			return -1;
+		}
+
+		for (k = 1; k < write_size/2; ++k) {
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "unexpected block read");
+				return -1;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+
+	return 0;
+}
+
+static void tests(void)
+{
+	int ret = 0;
+
+	tst_res(TINFO, "OFD read locks vs OFD write locks");
+	ret = test_fn(thread_fn_ofd_wlock, thread_fn_ofd_rlock);
+	if (ret == 0)
+		tst_res(TPASS, "access between threads synchronized");
+
+	ret = 0;
+	write_flag_cnt = 0;
+	tst_res(TINFO, "OFD write locks vs POSIX write locks");
+	ret = test_fn(thread_fn_ofd_wlock, thread_fn_posix_wlock);
+	if (ret == 0)
+		tst_res(TPASS, "access between threads synchronized");
+
+	ret = 0;
+	write_flag_cnt = 0;
+	tst_res(TINFO, "OFD read locks vs POSIX write locks");
+	ret = test_fn(thread_fn_ofd_rlock, thread_fn_posix_wlock);
+	if (ret == 0)
+		tst_res(TPASS, "access between threads synchronized");
+
+	ret = 0;
+	write_flag_cnt = 0;
+	tst_res(TINFO, "OFD write locks vs POSIX read locks");
+	ret = test_fn(thread_fn_ofd_wlock, thread_fn_posix_rlock);
+	if (ret == 0)
+		tst_res(TPASS, "access between threads synchronized");
+
+	ret = 0;
+	write_flag_cnt = 0;
+	tst_res(TINFO, "OFD write locks vs OFD write locks");
+	ret = test_fn(thread_fn_ofd_wlock, thread_fn_ofd_wlock);
+	if (ret == 0)
+		tst_res(TPASS, "access between threads synchronized");
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15.0",
+	.needs_tmpdir = 1,
+	.test_all = tests,
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
>  testcases/kernel/syscalls/fcntl/Makefile  |   3 +
>  testcases/kernel/syscalls/fcntl/fcntl36.c | 315 ++++++++++++++++++++++++++++++

You have to add a record to the corresponding runtest file as well.

> diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
> index d78dd72..ae37214 100644
> --- a/testcases/kernel/syscalls/fcntl/Makefile
> +++ b/testcases/kernel/syscalls/fcntl/Makefile
> @@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
>  fcntl34: LDLIBS += -lpthread
>  fcntl34_64: LDLIBS += -lpthread
>  
> +fcntl36: LDLIBS += -lpthread
> +fcntl36_64: LDLIBS += -lpthread
> +
>  include $(top_srcdir)/include/mk/testcases.mk
>  include $(abs_srcdir)/../utils/newer_64.mk
>  
> diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
> new file mode 100644
> index 0000000..4c55279
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
> @@ -0,0 +1,315 @@
> +/*
> + * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Xiong Zhou <xzhou@redhat.com>
> + *
> + * This is testing OFD locks racing with POSIX locks:
> + *
> + *	OFD read  lock   vs   OFD   write lock
> + *	OFD read  lock   vs   POSIX write lock
> + *	OFD write lock   vs   POSIX write lock
> + *	OFD write lock   vs   POSIX read  lock
> + *	OFD write lock   vs   OFD   write lock

What exactly are we trying to test here? This description should be more
verbose.

> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <pthread.h>
> +#include <sched.h>
> +#include <errno.h>
> +
> +#include "lapi/fcntl.h"
> +#include "tst_safe_pthread.h"
> +#include "tst_test.h"
> +
> +static int thread_cnt;
> +static const int max_thread_cnt = 32;
> +static const char fname[] = "tst_ofd_locks";
> +static int write_flag_cnt;
> +const int write_size = 4096;
> +
> +static void setup(void)
> +{
> +	thread_cnt = tst_ncpus_conf() * 3;
> +	if (thread_cnt > max_thread_cnt)
> +		thread_cnt = max_thread_cnt;
> +}
> +
> +static void spawn_threads(pthread_t *id, void *(*thread_fn)(void *))
> +{
> +	intptr_t i;
> +
> +	for (i = 0; i < thread_cnt; ++i)
> +		SAFE_PTHREAD_CREATE(id + i, NULL, thread_fn, (void *)i);
> +}
> +
> +static void wait_threads(pthread_t *id)
> +{
> +	int i;
> +
> +	for (i = 0; i < thread_cnt; ++i)
> +		SAFE_PTHREAD_JOIN(id[i], NULL);
> +}

Now these two functions are a bit unconsitent since the thread_cnt is
passed in global variable while the id is passed as a parameter. It
would be a bit cleaner if we passed both as a parameter.

> +/* Loop OFD wlock writing data*/
> +static void *thread_fn_ofd_wlock(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	long start = (intptr_t)arg * write_size;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	int i = 0;
> +
> +	/* write with half writesize offset in the second spawning */
> +	start += (write_flag_cnt % 2) * write_size / 2;

Why do we do this?

I guess that it's for the case where we run OFD wlocks concurently and
we want them to overlap.

However there are a few problems with this approach, one is that
propagating pthread parameters via global variables is not safe. There
is no guarantee that all the threads has already finished setting the
start variable when we change it in the main thread. The clean solution
to this problem is to pack the paramters into a structure and pass it
to this function as a paramter, which would in turn mean that we have
to prepare an array of these structures in the main thread, we can do
that on the stack in the test function since we wait all the threads
before we leave it (otherwise we would end up using part of stack that
will be rewritten later).

If we move the loop that creates the threads to the test function we can
do something as:

struct param {
	long offset;
	long length;
};

...
void do_test(...)
{
	...

	struct param params[threads_cnt];

	for (i = 0; i < threads_cnt; i++) {
		params[i].offset = ...;
		params[i].lenght = ...;
		SAFE_PTHREAD_CREATE(id + i, NULL, fn, params);
	}

	...

	for (i = 0; i < threads_cnt; i++)
		SAFE_PTHREAD_JOIN(id[i]);

	...
}

> +	memset(buf, 'o', write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = start,
> +		.l_len    = write_size,
> +		.l_pid    = 0,
> +	};
> +
> +	struct timespec ts = {
> +               .tv_sec = 0,
> +               .tv_nsec = 50000000,
> +	};
> +
> +	for (; i < thread_cnt; i++) {
> +
> +		lck.l_type = F_WRLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		SAFE_LSEEK(fd, start, SEEK_SET);
> +		SAFE_WRITE(1, fd, buf, write_size);
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		nanosleep(&ts, NULL);
> +		sched_yield();
> +	}

I do not get why we loop thread_cnt times here at all and why do we call
the nanosleep() between the locking attempts.

> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* Loop POSIX wlock writing data*/
> +static void *thread_fn_posix_wlock(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	long start = (intptr_t)arg * write_size;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	int i = 0;
> +
> +	memset(buf, 'p', write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = start,
> +		.l_len    = write_size,
> +	};
> +
> +	for (; i < thread_cnt; i++) {
> +
> +		lck.l_type = F_WRLCK;
> +		SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +		SAFE_LSEEK(fd, start, SEEK_SET);
> +		SAFE_WRITE(1, fd, buf, write_size);
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* Loop OFD rlock reading data*/
> +static void *thread_fn_ofd_rlock(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	long start = (intptr_t)arg * write_size;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	int i = 0;
> +
> +	memset(buf, (intptr_t)arg, write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = start,
> +		.l_len    = write_size,
> +		.l_pid    = 0,
> +	};
> +
> +	for (; i < thread_cnt; i++) {
> +
> +		lck.l_type = F_RDLCK;
> +		if (fcntl(fd, F_OFD_SETLK, &lck) == -1) {
> +			/* can only fail with EAGAIN */
> +			if (errno != EAGAIN)
> +				tst_brk(TBROK, "errno not equal to EAGAIN");

In which case do we get EAGAIN?

Also the tst_brk should include TERRNO here.

> +		} else {
> +
> +			/* rlock acquired */
> +			SAFE_LSEEK(fd, start, SEEK_SET);
> +			SAFE_READ(0, fd, buf, write_size);


Shouldn't we check here that the whole block has the same value?

Since in the combination where we run writers vs readers the check that
the file has consistent content at the end of the run does not really
check anything.

Also each of the writers should write a random character into its block
and the value should change on each iteration. Then we can check that
the readers see only consistently filled blocks.

> +			lck.l_type = F_UNLCK;
> +			SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
> +		}
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* Loop POSIX rlock reading data */
> +static void *thread_fn_posix_rlock(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	long start = (intptr_t)arg * write_size;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	int i = 0;
> +
> +	memset(buf, (intptr_t)arg, write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = start,
> +		.l_len    = write_size,
> +	};
> +
> +	for (; i < thread_cnt; i++) {
> +
> +		lck.l_type = F_RDLCK;
> +		if (fcntl(fd, F_SETLK, &lck) == -1) {
> +			/* can only fail with EAGAIN */
> +			if (errno != EAGAIN)
> +				tst_brk(TBROK, "errno not equal to EAGAIN");
> +		} else {
> +
> +			/* rlock acquired */
> +			SAFE_LSEEK(fd, start, SEEK_SET);
> +			SAFE_READ(0, fd, buf, write_size);

And here as well.

> +			lck.l_type = F_UNLCK;
> +			SAFE_FCNTL(fd, F_SETLK, &lck);
> +		}
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +static int test_fn(void *f0(void *), void *f1(void *))
> +{
> +	intptr_t i;
> +	int k;
> +	pthread_t id0[thread_cnt];
> +	pthread_t id1[thread_cnt];
> +	unsigned char buf[write_size];
> +
> +	int fd = SAFE_OPEN(fname, O_CREAT | O_TRUNC | O_RDWR, 0600);
> +
> +	spawn_threads(id0, f0);
> +	write_flag_cnt++;
> +	spawn_threads(id1, f1);
> +
> +	wait_threads(id0);
> +	wait_threads(id1);
> +
> +	tst_res(TINFO, "Verifying file's data");
> +	SAFE_LSEEK(fd, 0, SEEK_SET);
> +
> +	for (i = 0; i <= thread_cnt; ++i) {
> +
> +		SAFE_READ(1, fd, buf, write_size/2);
> +
> +		if (buf[0] != 'o' && buf[0] != 'p') {
> +			tst_res(TFAIL, "unexpected data read");
> +			return -1;
> +		}
> +
> +		for (k = 1; k < write_size/2; ++k) {
> +			if (buf[k] != buf[0]) {
> +				tst_res(TFAIL, "unexpected block read");
> +				return -1;
> +			}
> +		}

This check is really needed only in a case that we run two writers
concurently, but I guess that doing it at the end of the test does not
harm.

> +	}
> +	SAFE_CLOSE(fd);
> +
> +	return 0;
> +}
> +
> +static void tests(void)
> +{
> +	int ret = 0;
> +
> +	tst_res(TINFO, "OFD read locks vs OFD write locks");
> +	ret = test_fn(thread_fn_ofd_wlock, thread_fn_ofd_rlock);
> +	if (ret == 0)
> +		tst_res(TPASS, "access between threads synchronized");
> +
> +	ret = 0;
> +	write_flag_cnt = 0;
> +	tst_res(TINFO, "OFD write locks vs POSIX write locks");
> +	ret = test_fn(thread_fn_ofd_wlock, thread_fn_posix_wlock);
> +	if (ret == 0)
> +		tst_res(TPASS, "access between threads synchronized");
> +
> +	ret = 0;
> +	write_flag_cnt = 0;
> +	tst_res(TINFO, "OFD read locks vs POSIX write locks");
> +	ret = test_fn(thread_fn_ofd_rlock, thread_fn_posix_wlock);
> +	if (ret == 0)
> +		tst_res(TPASS, "access between threads synchronized");
> +
> +	ret = 0;
> +	write_flag_cnt = 0;
> +	tst_res(TINFO, "OFD write locks vs POSIX read locks");
> +	ret = test_fn(thread_fn_ofd_wlock, thread_fn_posix_rlock);
> +	if (ret == 0)
> +		tst_res(TPASS, "access between threads synchronized");
> +
> +	ret = 0;
> +	write_flag_cnt = 0;
> +	tst_res(TINFO, "OFD write locks vs OFD write locks");
> +	ret = test_fn(thread_fn_ofd_wlock, thread_fn_ofd_wlock);
> +	if (ret == 0)
> +		tst_res(TPASS, "access between threads synchronized");

Can we split this into a separate tests? Just create a structure with
two funciton pointers and a string for a description and we can use
.test with an index into an array of these.

Also there is no reason to print the TPASS message here, why don't we do
that in the test_fn() function?

And there is absolutely no need to zero the ret variable as well and the
write_flag_cnt should be reset in the test_fn() where it's actually set.

> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "3.15.0",
                       ^
		       No need for the .0 at the end "3.15" is a valid
		       version string.
> +	.needs_tmpdir = 1,
> +	.test_all = tests,
> +	.setup = setup
> +};
> -- 
> 1.8.3.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.

 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 380 ++++++++++++++++++++++++++++++
 3 files changed, 385 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..18b5d02
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,380 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 2k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 2k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+const long write_size = 4096;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	int fd = SAFE_OPEN(fname, O_CREAT | O_TRUNC | O_RDWR, 0600);
+	char *s = NULL;
+
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+
+	s = SAFE_MALLOC(thread_cnt * write_size);
+	memset(s, 1, thread_cnt * write_size);
+	SAFE_WRITE(1, fd, s, thread_cnt * write_size);
+
+	SAFE_CLOSE(fd);
+	if (s) {
+		free(s);
+		s = NULL;
+	}
+}
+
+static void spawn_threads(int cnt, pthread_t *id0, pthread_t *id1,
+		void *(*thread_f0)(void *), void *(*thread_f1)(void *),
+		struct param *p0, struct param *p1)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		SAFE_PTHREAD_CREATE(id0 + i, NULL, thread_f0, (void *)&p0[i]);
+		SAFE_PTHREAD_CREATE(id1 + i, NULL, thread_f1, (void *)&p1[i]);
+	}
+}
+
+static void wait_threads(int cnt, pthread_t *id0, pthread_t *id1)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		SAFE_PTHREAD_JOIN(id0[i], NULL);
+		SAFE_PTHREAD_JOIN(id1[i], NULL);
+	}
+}
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	memset(buf, pa->cnt, write_size);
+
+	lck.l_type = F_WRLCK;
+	SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+	SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+	SAFE_WRITE(1, fd, buf, pa->length);
+
+	lck.l_type = F_UNLCK;
+	SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+	sched_yield();
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	memset(buf, pa->cnt, write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	lck.l_type = F_WRLCK;
+	SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+	SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+	SAFE_WRITE(1, fd, buf, pa->length);
+
+	lck.l_type = F_UNLCK;
+	SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+	sched_yield();
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	memset(buf, 0, write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	lck.l_type = F_RDLCK;
+	if (fcntl(fd, F_OFD_SETLKW, &lck) == -1) {
+
+		tst_brk(TBROK | TERRNO, "acquiring OFD read lock failed");
+	} else {
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > thread_cnt + 1) {
+				tst_res(TFAIL, "unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+
+			if ((i < pa->length / 2 && buf[i] != buf[0]) ||
+				(i >= pa->length / 2 &&
+					buf[i] != buf[pa->length / 2])) {
+				tst_res(TFAIL, "unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+	}
+
+	sched_yield();
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	memset(buf, 0, write_size);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	lck.l_type = F_RDLCK;
+	if (fcntl(fd, F_SETLKW, &lck) == -1) {
+
+		tst_brk(TBROK | TERRNO, "acquiring OFD read lock failed");
+	} else {
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > thread_cnt + 1) {
+				tst_res(TFAIL, "unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+
+			if ((i < pa->length / 2 && buf[i] != buf[0]) ||
+				(i >= pa->length / 2 &&
+					buf[i] != buf[pa->length / 2])) {
+				tst_res(TFAIL, "unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+	}
+
+	sched_yield();
+	SAFE_CLOSE(fd);
+
+	return NULL;
+}
+
+/* Test different functions and verify data */
+static int test_fn(void *f0(void *), void *f1(void *), char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	unsigned char buf[write_size];
+
+	setup();
+	tst_res(TINFO, msg);
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size;
+		p1[i].offset += write_size / 2;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+	}
+
+	spawn_threads(thread_cnt, id0, id1, f0, f1, p0, p1);
+	wait_threads(thread_cnt, id0, id1);
+
+	tst_res(TINFO, "Verifying file's data");
+	fd = SAFE_OPEN(fname, O_RDWR);
+	memset(buf, 0, write_size);
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+
+	for (i = 0; i <= thread_cnt; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/2);
+
+		for (k = 0; k < write_size/2; k++) {
+
+			if (buf[k] < 2 || buf[k] > thread_cnt + 1) {
+				if (i == 0 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "unexpected data, "
+					"offset %ld value %d",
+					i * write_size + k, buf[k]);
+				return -1;
+			}
+		}
+
+		for (k = 1; k < write_size/2; k++) {
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "unexpected block read");
+				return -1;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+	tst_res(TPASS, "access between threads synchronized");
+
+	return 0;
+}
+
+static void tests(unsigned int i)
+{
+	switch (i) {
+	case 0:
+		test_fn(fn_ofd_r, fn_ofd_w,
+			"OFD read locks vs OFD write locks");
+		break;
+	case 1:
+		test_fn(fn_ofd_w, fn_posix_w,
+			"OFD write locks vs POSIX write locks");
+		break;
+	case 2:
+		test_fn(fn_ofd_r, fn_posix_w,
+			"OFD read locks vs POSIX write locks");
+		break;
+	case 3:
+		test_fn(fn_posix_r, fn_ofd_w,
+			"OFD write locks vs POSIX read locks");
+		break;
+	case 4:
+		test_fn(fn_ofd_w, fn_ofd_w,
+			"OFD write locks vs OFD write locks");
+		break;
+	}
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = 5,
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v2] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
> +/*
> + * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Xiong Zhou <xzhou@redhat.com>
> + *
> + * This is testing OFD locks racing with POSIX locks:
> + *
> + *	OFD read  lock   vs   OFD   write lock
> + *	OFD read  lock   vs   POSIX write lock
> + *	OFD write lock   vs   POSIX write lock
> + *	OFD write lock   vs   POSIX read  lock
> + *	OFD write lock   vs   OFD   write lock
> + *
> + * For example:
> + *
> + * 	Init an file with preset values.
> + *
> + * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
> + * 		checking data read back, there should not be any surprise
> + * 		values and data should be consistent in a 2k block.
> + *
> + * 	Threads acquire OFD WRITE locks to write a 4k section start from 2k,
> + * 		writing different values in different threads.
> + *
> + * 	Check file data after racing, there should not be any surprise values
> + * 		and data should be consistent in a 2k block.
> + *
> + *
> + */
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <pthread.h>
> +#include <sched.h>
> +#include <errno.h>
> +
> +#include "lapi/fcntl.h"
> +#include "tst_safe_pthread.h"
> +#include "tst_test.h"
> +
> +static int thread_cnt;
> +static const int max_thread_cnt = 32;
> +static const char fname[] = "tst_ofd_posix_locks";
> +const long write_size = 4096;
> +
> +struct param {
> +	long offset;
> +	long length;
> +	long cnt;
> +};
> +
> +static void setup(void)
> +{
> +	int fd = SAFE_OPEN(fname, O_CREAT | O_TRUNC | O_RDWR, 0600);
> +	char *s = NULL;

There is not need to initialize the variable since it's set just a few
lines below.

> +	thread_cnt = tst_ncpus_conf() * 3;
> +	if (thread_cnt > max_thread_cnt)
> +		thread_cnt = max_thread_cnt;
> +
> +	s = SAFE_MALLOC(thread_cnt * write_size);
> +	memset(s, 1, thread_cnt * write_size);
> +	SAFE_WRITE(1, fd, s, thread_cnt * write_size);
> +
> +	SAFE_CLOSE(fd);
> +	if (s) {
> +		free(s);
> +		s = NULL;
> +	}

And there is no need both for the if (), since free(NULL) is no-op and
for the s = NULL since the variable gets out of scope anyway.

Moreover we do have tst_fill_file() function exactly for this purpose.

> +}
> +
> +static void spawn_threads(int cnt, pthread_t *id0, pthread_t *id1,
> +		void *(*thread_f0)(void *), void *(*thread_f1)(void *),
> +		struct param *p0, struct param *p1)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		SAFE_PTHREAD_CREATE(id0 + i, NULL, thread_f0, (void *)&p0[i]);
> +		SAFE_PTHREAD_CREATE(id1 + i, NULL, thread_f1, (void *)&p1[i]);
> +	}
> +}
> +
> +static void wait_threads(int cnt, pthread_t *id0, pthread_t *id1)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		SAFE_PTHREAD_JOIN(id0[i], NULL);
> +		SAFE_PTHREAD_JOIN(id1[i], NULL);
> +	}
> +}
> +
> +/* OFD write lock writing data*/
> +static void *fn_ofd_w(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +		.l_pid    = 0,
> +	};
> +
> +	memset(buf, pa->cnt, write_size);
> +
> +	lck.l_type = F_WRLCK;
> +	SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +	SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +	SAFE_WRITE(1, fd, buf, pa->length);
> +
> +	lck.l_type = F_UNLCK;
> +	SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +	sched_yield();
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* POSIX write lock writing data*/
> +static void *fn_posix_w(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	memset(buf, pa->cnt, write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +	};
> +
> +	lck.l_type = F_WRLCK;
> +	SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +	SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +	SAFE_WRITE(1, fd, buf, pa->length);
> +
> +	lck.l_type = F_UNLCK;
> +	SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +	sched_yield();
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* OFD read lock reading data*/
> +static void *fn_ofd_r(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int i;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	memset(buf, 0, write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +		.l_pid    = 0,
> +	};
> +
> +	lck.l_type = F_RDLCK;
> +	if (fcntl(fd, F_OFD_SETLKW, &lck) == -1) {
> +
> +		tst_brk(TBROK | TERRNO, "acquiring OFD read lock failed");
> +	} else {

We can use SAFE_FCNTL() here since the file is pre-allocated, right?

> +
> +		/* rlock acquired */
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_READ(1, fd, buf, pa->length);
> +
> +		/* Verifying data read */
> +		for (i = 0; i < pa->length; i++) {
> +
> +			if (buf[i] < 1 || buf[i] > thread_cnt + 1) {
> +				tst_res(TFAIL, "unexpected data "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +
> +			if ((i < pa->length / 2 && buf[i] != buf[0]) ||
> +				(i >= pa->length / 2 &&
> +					buf[i] != buf[pa->length / 2])) {
> +				tst_res(TFAIL, "unexpected data "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +		}
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
> +	}
> +
> +	sched_yield();
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}
> +
> +/* POSIX read lock reading data */
> +static void *fn_posix_r(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int i;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	memset(buf, 0, write_size);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +	};
> +
> +	lck.l_type = F_RDLCK;
> +	if (fcntl(fd, F_SETLKW, &lck) == -1) {
> +
> +		tst_brk(TBROK | TERRNO, "acquiring OFD read lock failed");
> +	} else {

And here as well.

> +		/* rlock acquired */
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_READ(1, fd, buf, pa->length);
> +
> +		/* Verifying data read */
> +		for (i = 0; i < pa->length; i++) {
> +
> +			if (buf[i] < 1 || buf[i] > thread_cnt + 1) {
> +				tst_res(TFAIL, "unexpected data, "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +
> +			if ((i < pa->length / 2 && buf[i] != buf[0]) ||
> +				(i >= pa->length / 2 &&
> +					buf[i] != buf[pa->length / 2])) {
> +				tst_res(TFAIL, "unexpected data, "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +		}
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_SETLK, &lck);
> +	}
> +
> +	sched_yield();
> +	SAFE_CLOSE(fd);
> +
> +	return NULL;
> +}

These functions do just one iteration now. We should run them in a loop
for some time in order stress the locks a bit. The easiest way would be
to declare global volatile flag and loop while it's set in these
functions, sleep in the main thread for a bit between spawn_threads()
and join_threads() then reset the flag and join the threads. We can even
add a test parameter for how long the test should run.

And if we do that we should as well change the value writers write on
each iteration otherwise we wouldn't test anything after first iteration
in the writer vs reader case. I would pass the cnt as it's done here but
will increment it each iteration and set it to 2 once it reaches 256 in
order to fit into a byte.

> +/* Test different functions and verify data */
> +static int test_fn(void *f0(void *), void *f1(void *), char *msg)

Again this should be just void since we do not use the returned value at
all.

> +{
> +	int i, k, fd;
> +	pthread_t id0[thread_cnt];
> +	pthread_t id1[thread_cnt];
> +	struct param p0[thread_cnt];
> +	struct param p1[thread_cnt];
> +	unsigned char buf[write_size];
> +
> +	setup();

This is not right, if we want to rewrite the test file before each test
the code to write the file should be here and the setup should contain
only the check for thread_cnt.

> +	tst_res(TINFO, msg);
> +
> +	for (i = 0; i < thread_cnt; i++) {
> +
> +		p0[i].offset = i * write_size;
> +		p0[i].length = write_size;
> +		p0[i].cnt = i + 2;
> +
> +		p1[i].offset = i * write_size;
> +		p1[i].offset += write_size / 2;
> +		p1[i].length = write_size;
> +		p1[i].cnt = i + 2;
> +	}
> +
> +	spawn_threads(thread_cnt, id0, id1, f0, f1, p0, p1);
> +	wait_threads(thread_cnt, id0, id1);
> +
> +	tst_res(TINFO, "Verifying file's data");
> +	fd = SAFE_OPEN(fname, O_RDWR);
> +	memset(buf, 0, write_size);

Here again, we actually fill the buffer with data and we make sure that
it has been filled (since we pass 1 to the SAFE_READ() as first
paramter) hence there is absolutely no need to initialize the buffer
here.

> +	SAFE_LSEEK(fd, 0, SEEK_SET);
> +
> +	for (i = 0; i <= thread_cnt; i++) {
> +
> +		SAFE_READ(1, fd, buf, write_size/2);
> +
> +		for (k = 0; k < write_size/2; k++) {
> +
> +			if (buf[k] < 2 || buf[k] > thread_cnt + 1) {
> +				if (i == 0 && buf[k] == 1)
> +					continue;
> +				tst_res(TFAIL, "unexpected data, "
> +					"offset %ld value %d",
> +					i * write_size + k, buf[k]);
> +				return -1;
> +			}
> +		}
> +
> +		for (k = 1; k < write_size/2; k++) {
> +			if (buf[k] != buf[0]) {
> +				tst_res(TFAIL, "unexpected block read");
> +				return -1;
> +			}
> +		}
> +	}
> +
> +	SAFE_CLOSE(fd);
> +	tst_res(TPASS, "access between threads synchronized");
> +
> +	return 0;
> +}
> +
> +static void tests(unsigned int i)
> +{
> +	switch (i) {
> +	case 0:
> +		test_fn(fn_ofd_r, fn_ofd_w,
> +			"OFD read locks vs OFD write locks");
> +		break;
> +	case 1:
> +		test_fn(fn_ofd_w, fn_posix_w,
> +			"OFD write locks vs POSIX write locks");
> +		break;
> +	case 2:
> +		test_fn(fn_ofd_r, fn_posix_w,
> +			"OFD read locks vs POSIX write locks");
> +		break;
> +	case 3:
> +		test_fn(fn_posix_r, fn_ofd_w,
> +			"OFD write locks vs POSIX read locks");
> +		break;
> +	case 4:
> +		test_fn(fn_ofd_w, fn_ofd_w,
> +			"OFD write locks vs OFD write locks");
> +		break;
> +	}
> +}

We may as well add these into a structure and use index into an array
instead of the switch, something as:

static struct tcase {
	void* (*fn1)(void *);
	void* (*fn2)(void *);
	const char *desc;
} tcases[] = {
	{fn_ofd_r, fn_ofd_w, "OFD read locks vs OFD write locks"},
	...
};

static void tests(unsigned int i)
{
	test(tcase[i].fn0, tcase[i].fn1, tcase[i].desc);
}

> +static struct tst_test test = {
> +	.min_kver = "3.15",
> +	.needs_tmpdir = 1,
> +	.test = tests,
> +	.tcnt = 5,
> +	.setup = setup
> +};

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Testing OFD locks racing with POSIX locks:

  OFD read  lock   vs   OFD   write lock
  OFD read  lock   vs   POSIX write lock
  OFD write lock   vs   POSIX write lock
  OFD write lock   vs   POSIX read  lock
  OFD write lock   vs   OFD   write lock

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v3:
  Use tst_fill_file.
  Loop in test function.
  Other fix.

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.

 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 386 ++++++++++++++++++++++++++++++
 3 files changed, 391 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..7aea434
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,386 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 2k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 2k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static volatile int loop_flag;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+static const long write_size = 4096;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+#define debug
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (1) {
+
+		if (loop_flag == 1)
+			break;
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (1) {
+
+		if (loop_flag == 1)
+			break;
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (1) {
+
+		if (loop_flag == 1)
+			break;
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+
+			long tmp = pa->length / 2;
+
+			if ((i < tmp && buf[i] != buf[0]) ||
+			    (i >= tmp && buf[i] != buf[tmp])) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (1) {
+
+		if (loop_flag == 1)
+			break;
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+
+			long tmp = pa->length / 2;
+
+			if ((i < tmp && buf[i] != buf[0]) ||
+			    (i >= tmp && buf[i] != buf[tmp])) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* Test different functions and verify data */
+static void test_fn(void *f0(void *), void *f1(void *), const char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	unsigned char buf[write_size];
+
+	if (tst_fill_file(fname, 1, write_size, thread_cnt)) {
+		tst_brk(TBROK, "Failed to create tst file");
+	}
+
+	tst_res(TINFO, msg);
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size;
+		p1[i].offset += write_size / 2;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+	}
+
+	loop_flag = 0;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
+		SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
+	}
+
+	sleep(3);
+	loop_flag = 1;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_JOIN(id0[i], NULL);
+		SAFE_PTHREAD_JOIN(id1[i], NULL);
+	}
+
+	tst_res(TINFO, "Verifying file's data");
+	fd = SAFE_OPEN(fname, O_RDWR);
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+
+	for (i = 0; i < thread_cnt * 2; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/2);
+
+		for (k = 0; k < write_size/2; k++) {
+
+			if (buf[k] < 2 || buf[k] > 254) {
+
+				if (i == 0 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					i * write_size / 2 + k, buf[k]);
+				return;
+			}
+		}
+
+		for (k = 1; k < write_size/2; k++) {
+
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "Unexpected block read");
+				return;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+	tst_res(TPASS, "Access between threads synchronized");
+}
+
+static struct tcase {
+
+	void *(*fn0)(void *);
+	void *(*fn1)(void *);
+	const char *desc;
+
+} tcases[] = {
+
+	{fn_ofd_r,   fn_ofd_w,   "OFD read locks vs OFD write locks"},
+	{fn_ofd_w,   fn_posix_w, "OFD write locks vs POSIX write locks"},
+	{fn_ofd_r,   fn_posix_w, "OFD read locks vs POSIX write locks"},
+	{fn_posix_r, fn_ofd_w,   "OFD write locks vs POSIX read locks"},
+	{fn_ofd_w,   fn_ofd_w,   "OFD write locks vs OFD write locks"},
+};
+
+static void tests(unsigned int i)
+{
+	test_fn(tcases[i].fn0, tcases[i].fn1, tcases[i].desc);
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = 5,
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v3] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
>  runtest/syscalls                          |   2 +
>  testcases/kernel/syscalls/fcntl/Makefile  |   3 +
>  testcases/kernel/syscalls/fcntl/fcntl36.c | 386 ++++++++++++++++++++++++++++++

Don't forget about .gitignore entry as well.

...

> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <pthread.h>
> +#include <sched.h>
> +#include <errno.h>
> +
> +#include "lapi/fcntl.h"
> +#include "tst_safe_pthread.h"
> +#include "tst_test.h"
> +
> +static int thread_cnt;
> +static volatile int loop_flag;
> +static const int max_thread_cnt = 32;
> +static const char fname[] = "tst_ofd_posix_locks";
> +static const long write_size = 4096;
> +
> +struct param {
> +	long offset;
> +	long length;
> +	long cnt;
> +};
> +
> +static void setup(void)
> +{
> +	thread_cnt = tst_ncpus_conf() * 3;
> +	if (thread_cnt > max_thread_cnt)
> +		thread_cnt = max_thread_cnt;
> +}
> +
> +#define debug
             ^
Forget to remove this

> +/* OFD write lock writing data*/
> +static void *fn_ofd_w(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	long wt = pa->cnt;
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +		.l_pid    = 0,
> +	};
> +
> +	while (1) {
> +
> +		if (loop_flag == 1)
> +			break;

Common, why not just while (loop_flag) ?

And we set the loop_flag to 1 before we spawn the threads and then reset
it to 0 after we do the sleep.

> +		memset(buf, wt, write_size);
> +
> +		lck.l_type = F_WRLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_WRITE(1, fd, buf, pa->length);
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		wt++;
> +		if (wt >= 255)
> +			wt = pa->cnt;
> +
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +	return NULL;
> +}
> +
> +/* POSIX write lock writing data*/
> +static void *fn_posix_w(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	long wt = pa->cnt;
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +	};
> +
> +	while (1) {
> +
> +		if (loop_flag == 1)
> +			break;

Here as well.

> +		memset(buf, wt, write_size);
> +
> +		lck.l_type = F_WRLCK;
> +		SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_WRITE(1, fd, buf, pa->length);
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +		wt++;
> +		if (wt >= 255)
> +			wt = pa->cnt;
> +
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +	return NULL;
> +}
> +
> +/* OFD read lock reading data*/
> +static void *fn_ofd_r(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int i;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +		.l_pid    = 0,
> +	};
> +
> +	while (1) {
> +
> +		if (loop_flag == 1)
> +			break;

And here as well.

> +		memset(buf, 0, write_size);
> +
> +		lck.l_type = F_RDLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		/* rlock acquired */
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_READ(1, fd, buf, pa->length);
> +
> +		/* Verifying data read */
> +		for (i = 0; i < pa->length; i++) {
> +
> +			if (buf[i] < 1 || buf[i] > 254) {
> +
> +				tst_res(TFAIL, "Unexpected data "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +
> +			long tmp = pa->length / 2;

We can simplify this with:
			int j = i / (pa->length/2);

			if (buf[i] != buf[j]) {
				...
			}

> +			if ((i < tmp && buf[i] != buf[0]) ||
> +			    (i >= tmp && buf[i] != buf[tmp])) {
> +
> +				tst_res(TFAIL, "Unexpected data "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +		}
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
> +
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +	return NULL;
> +}
> +
> +/* POSIX read lock reading data */
> +static void *fn_posix_r(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int i;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +	};
> +
> +	while (1) {
> +
> +		if (loop_flag == 1)
> +			break;

Here as well.

> +		memset(buf, 0, write_size);
> +
> +		lck.l_type = F_RDLCK;
> +		SAFE_FCNTL(fd, F_SETLKW, &lck);
> +
> +		/* rlock acquired */
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_READ(1, fd, buf, pa->length);
> +
> +		/* Verifying data read */
> +		for (i = 0; i < pa->length; i++) {
> +
> +			if (buf[i] < 1 || buf[i] > 254) {
> +
> +				tst_res(TFAIL, "Unexpected data, "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +
> +			long tmp = pa->length / 2;

And here as well.

> +			if ((i < tmp && buf[i] != buf[0]) ||
> +			    (i >= tmp && buf[i] != buf[tmp])) {
> +
> +				tst_res(TFAIL, "Unexpected data, "
> +					"offset %ld value %d",
> +					pa->offset + i, buf[i]);
> +				break;
> +			}
> +		}
> +
> +		lck.l_type = F_UNLCK;
> +		SAFE_FCNTL(fd, F_SETLK, &lck);
> +
> +		sched_yield();
> +	}
> +
> +	SAFE_CLOSE(fd);
> +	return NULL;
> +}
> +
> +/* Test different functions and verify data */
> +static void test_fn(void *f0(void *), void *f1(void *), const char *msg)
> +{
> +	int i, k, fd;
> +	pthread_t id0[thread_cnt];
> +	pthread_t id1[thread_cnt];
> +	struct param p0[thread_cnt];
> +	struct param p1[thread_cnt];
> +	unsigned char buf[write_size];
> +
> +	if (tst_fill_file(fname, 1, write_size, thread_cnt)) {
> +		tst_brk(TBROK, "Failed to create tst file");
> +	}

No need for the curly braces around single line statement here.

> +	tst_res(TINFO, msg);
> +
> +	for (i = 0; i < thread_cnt; i++) {
> +
> +		p0[i].offset = i * write_size;
> +		p0[i].length = write_size;
> +		p0[i].cnt = i + 2;
> +
> +		p1[i].offset = i * write_size;
> +		p1[i].offset += write_size / 2;
> +		p1[i].length = write_size;
> +		p1[i].cnt = i + 2;
> +	}
> +
> +	loop_flag = 0;
> +	for (i = 0; i < thread_cnt; i++) {
> +		SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
> +		SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
> +	}
> +
> +	sleep(3);

I would go fo 1s for default run, but that is very minor.

> +	loop_flag = 1;
> +	for (i = 0; i < thread_cnt; i++) {
> +		SAFE_PTHREAD_JOIN(id0[i], NULL);
> +		SAFE_PTHREAD_JOIN(id1[i], NULL);
> +	}
> +
> +	tst_res(TINFO, "Verifying file's data");
> +	fd = SAFE_OPEN(fname, O_RDWR);
> +	SAFE_LSEEK(fd, 0, SEEK_SET);

We do not write to the file, so we may as well open it RDONLY and since
we are opening it we don't have to seek it to the start.

> +	for (i = 0; i < thread_cnt * 2; i++) {
> +
> +		SAFE_READ(1, fd, buf, write_size/2);
> +
> +		for (k = 0; k < write_size/2; k++) {
> +
> +			if (buf[k] < 2 || buf[k] > 254) {
> +
> +				if (i == 0 && buf[k] == 1)
> +					continue;
> +				tst_res(TFAIL, "Unexpected data, "
> +					"offset %ld value %d",
> +					i * write_size / 2 + k, buf[k]);
> +				return;
> +			}
> +		}
> +
> +		for (k = 1; k < write_size/2; k++) {
> +
> +			if (buf[k] != buf[0]) {
> +				tst_res(TFAIL, "Unexpected block read");
> +				return;
> +			}
> +		}
> +	}

We should close the fd before the return here.

> +	SAFE_CLOSE(fd);
> +	tst_res(TPASS, "Access between threads synchronized");

And here we would print TPASS even if reader thread failed the test. So
we should either change tst_res() to tst_brk() in the reader thread
functions so that we exit the test if we read back wrong data. Or setup
a global failure counter and print TPASS only if the counter was not set
from one of the reader threads. Or propagate the failure from the tread
return value (the one you get from SAFE_JOIN()).

> +}
> +
> +static struct tcase {
> +
> +	void *(*fn0)(void *);
> +	void *(*fn1)(void *);
> +	const char *desc;
> +
> +} tcases[] = {
> +
> +	{fn_ofd_r,   fn_ofd_w,   "OFD read locks vs OFD write locks"},
> +	{fn_ofd_w,   fn_posix_w, "OFD write locks vs POSIX write locks"},
> +	{fn_ofd_r,   fn_posix_w, "OFD read locks vs POSIX write locks"},
> +	{fn_posix_r, fn_ofd_w,   "OFD write locks vs POSIX read locks"},
> +	{fn_ofd_w,   fn_ofd_w,   "OFD write locks vs OFD write locks"},
> +};
> +
> +static void tests(unsigned int i)
> +{
> +	test_fn(tcases[i].fn0, tcases[i].fn1, tcases[i].desc);
> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "3.15",
> +	.needs_tmpdir = 1,
> +	.test = tests,
> +	.tcnt = 5,
                ^
		ARRAY_SIZE(tcases) is safer here, that way it will be
		correct even if we add/remove tests in the array.
> +	.setup = setup
> +};
> -- 
> 1.8.3.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

On Tue, Aug 22, 2017 at 04:23:57PM +0200, Cyril Hrubis wrote:
> Hi!
> >  runtest/syscalls                          |   2 +
> >  testcases/kernel/syscalls/fcntl/Makefile  |   3 +
> >  testcases/kernel/syscalls/fcntl/fcntl36.c | 386 ++++++++++++++++++++++++++++++
> 
> Don't forget about .gitignore entry as well.

Ha, now i found the right one, :)

> 
> ...
> 
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
...
> > +		SAFE_READ(1, fd, buf, pa->length);
> > +
> > +		/* Verifying data read */
> > +		for (i = 0; i < pa->length; i++) {
> > +
> > +			if (buf[i] < 1 || buf[i] > 254) {
> > +
> > +				tst_res(TFAIL, "Unexpected data "
> > +					"offset %ld value %d",
> > +					pa->offset + i, buf[i]);
> > +				break;
> > +			}
> > +
> > +			long tmp = pa->length / 2;
> 
> We can simplify this with:
> 			int j = i / (pa->length/2);
> 
> 			if (buf[i] != buf[j]) {
> 				...
> 			}

After checking, this is not what I meant to check here about
this data verifying.

We could read 4k data that first half 2k of it have the same
value and the another half 2k have a different value.

> 
> > +			if ((i < tmp && buf[i] != buf[0]) ||
> > +			    (i >= tmp && buf[i] != buf[tmp])) {
> > +
> > +				tst_res(TFAIL, "Unexpected data "
> > +					"offset %ld value %d",
> > +					pa->offset + i, buf[i]);
> > +				break;
> > +			}
> > +		}
> > +
...
> > +	if (tst_fill_file(fname, 1, write_size, thread_cnt)) {
> > +		tst_brk(TBROK, "Failed to create tst file");
> > +	}
> 
> No need for the curly braces around single line statement here.

I thought it's safer for a macro. Now it's one line function in
the macro, not hurting to remove the braces.

Thanks for reviewing!
Xiong

[LTP] [PATCH v3] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
> > > +	if (tst_fill_file(fname, 1, write_size, thread_cnt)) {
> > > +		tst_brk(TBROK, "Failed to create tst file");
> > > +	}
> > 
> > No need for the curly braces around single line statement here.
> 
> I thought it's safer for a macro. Now it's one line function in
> the macro, not hurting to remove the braces.

Even macros that have several lines are safe in LTP since we use the
do { ... } while (0) idiom.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Testing OFD locks racing with POSIX locks:

  OFD read  lock   vs   OFD   write lock
  OFD read  lock   vs   POSIX write lock
  OFD write lock   vs   POSIX write lock
  OFD write lock   vs   POSIX read  lock
  OFD write lock   vs   OFD   write lock

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v4:
  Add .gitignore entry.
  Count failures in reader, then check it in main loop.
  Other fix.

v3:
  Use tst_fill_file.
  Loop in test function.
  Other fix.

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.

 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/.gitignore      |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 378 ++++++++++++++++++++++++++++++
 4 files changed, 385 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 2a7026d..32193e1 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -234,6 +234,8 @@
 /fcntl/fcntl34_64
 /fcntl/fcntl35
 /fcntl/fcntl35_64
+/fcntl/fcntl36
+/fcntl/fcntl36_64
 /fdatasync/fdatasync01
 /fdatasync/fdatasync02
 /flistxattr/flistxattr01
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..1afa278
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,378 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 2k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 2k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static int fail_cnt = 0;
+static volatile int loop_flag = 1;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+static const long write_size = 4096;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+
+			long tmp = pa->length / 2;
+
+			if ((i < tmp && buf[i] != buf[0]) ||
+			    (i >= tmp && buf[i] != buf[tmp])) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+
+			long tmp = pa->length / 2;
+
+			if ((i < tmp && buf[i] != buf[0]) ||
+			    (i >= tmp && buf[i] != buf[tmp])) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* Test different functions and verify data */
+static void test_fn(void *f0(void *), void *f1(void *), const char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	unsigned char buf[write_size];
+
+	if (tst_fill_file(fname, 1, write_size, thread_cnt))
+		tst_brk(TBROK, "Failed to create tst file");
+
+	tst_res(TINFO, msg);
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size;
+		p1[i].offset += write_size / 2;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+	}
+
+	loop_flag = 1;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
+		SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
+	}
+
+	sleep(1);
+	loop_flag = 0;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_JOIN(id0[i], NULL);
+		SAFE_PTHREAD_JOIN(id1[i], NULL);
+	}
+
+	tst_res(TINFO, "Verifying file's data");
+	fd = SAFE_OPEN(fname, O_RDONLY);
+
+	for (i = 0; i < thread_cnt * 2; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/2);
+
+		for (k = 0; k < write_size/2; k++) {
+
+			if (buf[k] < 2 || buf[k] > 254) {
+
+				if (i == 0 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					i * write_size / 2 + k, buf[k]);
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+
+		for (k = 1; k < write_size/2; k++) {
+
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "Unexpected block read");
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+	if (fail_cnt == 0)
+		tst_res(TPASS, "Access between threads synchronized");
+}
+
+static struct tcase {
+
+	void *(*fn0)(void *);
+	void *(*fn1)(void *);
+	const char *desc;
+
+} tcases[] = {
+
+	{fn_ofd_r,   fn_ofd_w,   "OFD read locks vs OFD write locks"},
+	{fn_ofd_w,   fn_posix_w, "OFD write locks vs POSIX write locks"},
+	{fn_ofd_r,   fn_posix_w, "OFD read locks vs POSIX write locks"},
+	{fn_posix_r, fn_ofd_w,   "OFD write locks vs POSIX read locks"},
+	{fn_ofd_w,   fn_ofd_w,   "OFD write locks vs OFD write locks"},
+};
+
+static void tests(unsigned int i)
+{
+	test_fn(tcases[i].fn0, tcases[i].fn1, tcases[i].desc);
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v3] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> ...
> > > +		SAFE_READ(1, fd, buf, pa->length);
> > > +
> > > +		/* Verifying data read */
> > > +		for (i = 0; i < pa->length; i++) {
> > > +
> > > +			if (buf[i] < 1 || buf[i] > 254) {
> > > +
> > > +				tst_res(TFAIL, "Unexpected data "
> > > +					"offset %ld value %d",
> > > +					pa->offset + i, buf[i]);
> > > +				break;
> > > +			}
> > > +
> > > +			long tmp = pa->length / 2;
> > 
> > We can simplify this with:
> > 			int j = i / (pa->length/2);
> > 
> > 			if (buf[i] != buf[j]) {
> > 				...
> > 			}
> 
> After checking, this is not what I meant to check here about
> this data verifying.
> 
> We could read 4k data that first half 2k of it have the same
> value and the another half 2k have a different value.

Ah right, made a silly mistake the correct formula is:

	int j = (i / (pa->length/2)) * pa->length/2;

The previous one was yielding incorrect result for i >= pa->length/2.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Testing OFD locks racing with POSIX locks:

  OFD read  lock   vs   OFD   write lock
  OFD read  lock   vs   POSIX write lock
  OFD write lock   vs   POSIX write lock
  OFD write lock   vs   POSIX read  lock
  OFD write lock   vs   OFD   write lock

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v5:
  Optimize data checking.

v4:
  Add .gitignore entry.
  Count failures in reader, then check it in main loop.
  Other fix.

v3:
  Use tst_fill_file.
  Loop in test function.
  Other fix.

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.
 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/.gitignore      |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 376 ++++++++++++++++++++++++++++++
 4 files changed, 383 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 2a7026d..32193e1 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -234,6 +234,8 @@
 /fcntl/fcntl34_64
 /fcntl/fcntl35
 /fcntl/fcntl35_64
+/fcntl/fcntl36
+/fcntl/fcntl36_64
 /fdatasync/fdatasync01
 /fdatasync/fdatasync02
 /flistxattr/flistxattr01
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..7bd001a
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,376 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 2k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 2k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static int fail_cnt = 0;
+static volatile int loop_flag = 1;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+static const long write_size = 4096;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, write_size);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+
+			int j = (i / (pa->length/2)) * pa->length/2;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	unsigned char buf[write_size];
+	struct param *pa = (struct param *)arg;
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, write_size);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+
+			int j = (i / (pa->length/2)) * pa->length/2;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_cnt++;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* Test different functions and verify data */
+static void test_fn(void *f0(void *), void *f1(void *), const char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	unsigned char buf[write_size];
+
+	if (tst_fill_file(fname, 1, write_size, thread_cnt))
+		tst_brk(TBROK, "Failed to create tst file");
+
+	tst_res(TINFO, msg);
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size;
+		p1[i].offset += write_size / 2;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+	}
+
+	loop_flag = 1;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
+		SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
+	}
+
+	sleep(1);
+	loop_flag = 0;
+	for (i = 0; i < thread_cnt; i++) {
+		SAFE_PTHREAD_JOIN(id0[i], NULL);
+		SAFE_PTHREAD_JOIN(id1[i], NULL);
+	}
+
+	tst_res(TINFO, "Verifying file's data");
+	fd = SAFE_OPEN(fname, O_RDONLY);
+
+	for (i = 0; i < thread_cnt * 2; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/2);
+
+		for (k = 0; k < write_size/2; k++) {
+
+			if (buf[k] < 2 || buf[k] > 254) {
+
+				if (i == 0 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "Unexpected data, "
+					"offset %ld value %d",
+					i * write_size / 2 + k, buf[k]);
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+
+		for (k = 1; k < write_size/2; k++) {
+
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "Unexpected block read");
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+	if (fail_cnt == 0)
+		tst_res(TPASS, "Access between threads synchronized");
+}
+
+static struct tcase {
+
+	void *(*fn0)(void *);
+	void *(*fn1)(void *);
+	const char *desc;
+
+} tcases[] = {
+
+	{fn_ofd_r,   fn_ofd_w,   "OFD read locks vs OFD write locks"},
+	{fn_ofd_w,   fn_posix_w, "OFD write locks vs POSIX write locks"},
+	{fn_ofd_r,   fn_posix_w, "OFD read locks vs POSIX write locks"},
+	{fn_posix_r, fn_ofd_w,   "OFD write locks vs POSIX read locks"},
+	{fn_ofd_w,   fn_ofd_w,   "OFD write locks vs OFD write locks"},
+};
+
+static void tests(unsigned int i)
+{
+	test_fn(tcases[i].fn0, tcases[i].fn1, tcases[i].desc);
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v5] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
> +/* OFD write lock writing data*/
> +static void *fn_ofd_w(void *arg)
> +{
> +	unsigned char buf[write_size];
> +	struct param *pa = (struct param *)arg;
> +	int fd = SAFE_OPEN(fname, O_RDWR);
> +	long wt = pa->cnt;
> +
> +	struct flock64 lck = {
> +		.l_whence = SEEK_SET,
> +		.l_start  = pa->offset,
> +		.l_len    = pa->length,
> +		.l_pid    = 0,
> +	};
> +
> +	while (loop_flag) {
> +
> +		memset(buf, wt, write_size);
> +
> +		lck.l_type = F_WRLCK;
> +		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
> +
> +		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
> +		SAFE_WRITE(1, fd, buf, pa->length);

Just a minor note, we use both write_size and pa->length randomly here
and in all of the test functions. It would be a much cleaner to use only
pa->lenght instead.

> +/* Test different functions and verify data */
> +static void test_fn(void *f0(void *), void *f1(void *), const char *msg)
> +{
> +	int i, k, fd;
> +	pthread_t id0[thread_cnt];
> +	pthread_t id1[thread_cnt];
> +	struct param p0[thread_cnt];
> +	struct param p1[thread_cnt];
> +	unsigned char buf[write_size];
> +
> +	if (tst_fill_file(fname, 1, write_size, thread_cnt))
> +		tst_brk(TBROK, "Failed to create tst file");
> +
> +	tst_res(TINFO, msg);
> +
> +	for (i = 0; i < thread_cnt; i++) {
> +
> +		p0[i].offset = i * write_size;
> +		p0[i].length = write_size;
> +		p0[i].cnt = i + 2;
> +
> +		p1[i].offset = i * write_size;
> +		p1[i].offset += write_size / 2;
> +		p1[i].length = write_size;
> +		p1[i].cnt = i + 2;
> +	}

We should reset the fail_cnt here. And it would be safer to rename it to
fail_flag and set it to 1 on a failure because if we are terribly
unlucky the incements can overflow and we may as well end up with a
zero.

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v6] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Testing OFD locks racing with POSIX locks:

  OFD read  lock   vs   OFD   write lock
  OFD read  lock   vs   POSIX write lock
  OFD write lock   vs   POSIX write lock
  OFD write lock   vs   POSIX read  lock
  OFD write lock   vs   OFD   write lock
  OFD   r/w locks  vs   POSIX write locks
  POSIX r/w locks  vs   OFD   write locks
  OFD   r/w locks  vs   POSIX read  locks
  POSIX r/w locks  vs   OFD   read  locks
  OFD   r/w locks  vs   POSIX r/w   locks

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v6:
  Use param->length in thread_fn as r/w size instead of
write_size directly, thanks to C99.

  Reset fail_flag after each sub case.

  Race up to 4 bunches of threads acquiring these 4 kinds of locks,
not just only 2 of them. (kind crazy)

v5:
  Optimize data checking.

v4:
  Add .gitignore entry.
  Count failures in reader, then check it in main loop.
  Other fix.

v3:
  Use tst_fill_file.
  Loop in test function.
  Other fix.

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.

 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/.gitignore      |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 429 ++++++++++++++++++++++++++++++
 4 files changed, 436 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 2a7026d..32193e1 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -234,6 +234,8 @@
 /fcntl/fcntl34_64
 /fcntl/fcntl35
 /fcntl/fcntl35_64
+/fcntl/fcntl36
+/fcntl/fcntl36_64
 /fdatasync/fdatasync01
 /fdatasync/fdatasync02
 /flistxattr/flistxattr01
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..e1205e2
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,429 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ *	OFD   r/w locks vs POSIX write locks
+ *	POSIX r/w locks vs OFD   write locks
+ *	OFD   r/w locks vs POSIX read locks
+ *	POSIX r/w locks vs OFD   read locks
+ *
+ *	OFD   r/w locks vs POSIX r/w locks
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 1k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 1k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static int fail_flag = 0;
+static volatile int loop_flag = 1;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+static const long write_size = 4096;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, pa->length);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, pa->length);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, pa->length);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+
+			int j = (i / (pa->length/4)) * pa->length/4;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, pa->length);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+
+			int j = (i / (pa->length/4)) * pa->length/4;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* Test different functions and verify data */
+static void test_fn(void *f0(void *), void *f1(void *),
+		    void *f2(void *), void *f3(void *), const char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	pthread_t id2[thread_cnt];
+	pthread_t id3[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	struct param p2[thread_cnt];
+	struct param p3[thread_cnt];
+	unsigned char buf[write_size];
+
+	if (tst_fill_file(fname, 1, write_size, thread_cnt + 1))
+		tst_brk(TBROK, "Failed to create tst file");
+
+	tst_res(TINFO, msg);
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size + write_size / 4;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+
+		p2[i].offset = i * write_size + write_size / 2;
+		p2[i].length = write_size;
+		p2[i].cnt = i + 2;
+
+		p3[i].offset = i * write_size + write_size * 3 / 4;
+		p3[i].length = write_size;
+		p3[i].cnt = i + 2;
+	}
+
+	fail_flag = 0;
+	loop_flag = 1;
+
+	for (i = 0; i < thread_cnt; i++) {
+		if (f0 != NULL)
+			SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
+		if (f1 != NULL)
+			SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
+		if (f2 != NULL)
+			SAFE_PTHREAD_CREATE(id2 + i, NULL, f2, (void *)&p2[i]);
+		if (f3 != NULL)
+			SAFE_PTHREAD_CREATE(id3 + i, NULL, f3, (void *)&p3[i]);
+	}
+
+	sleep(1);
+	loop_flag = 0;
+
+	for (i = 0; i < thread_cnt; i++) {
+		if (f0 != NULL)
+			SAFE_PTHREAD_JOIN(id0[i], NULL);
+		if (f1 != NULL)
+			SAFE_PTHREAD_JOIN(id1[i], NULL);
+		if (f2 != NULL)
+			SAFE_PTHREAD_JOIN(id2[i], NULL);
+		if (f3 != NULL)
+			SAFE_PTHREAD_JOIN(id3[i], NULL);
+	}
+
+	fd = SAFE_OPEN(fname, O_RDONLY);
+
+	for (i = 0; i < thread_cnt * 4; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/4);
+
+		for (k = 0; k < write_size/4; k++) {
+
+			if (buf[k] < 2 || buf[k] > 254) {
+
+				if (i < 3 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					i * write_size / 4 + k, buf[k]);
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+
+		for (k = 1; k < write_size/4; k++) {
+
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "Unexpected block read");
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+	}
+
+	SAFE_CLOSE(fd);
+	if (fail_flag == 0)
+		tst_res(TPASS, "Access between threads synchronized");
+}
+
+static struct tcase {
+
+	void *(*fn0)(void *);
+	void *(*fn1)(void *);
+	void *(*fn2)(void *);
+	void *(*fn3)(void *);
+	const char *desc;
+
+} tcases[] = {
+
+	{fn_ofd_r,   fn_ofd_w,
+		NULL, NULL, "OFD read  locks vs OFD   write locks"},
+	{fn_ofd_w,   fn_posix_w,
+		NULL, NULL, "OFD write locks vs POSIX write locks"},
+	{fn_ofd_r,   fn_posix_w,
+		NULL, NULL, "OFD read  locks vs POSIX write locks"},
+	{fn_posix_r, fn_ofd_w,
+		NULL, NULL, "OFD write locks vs POSIX read  locks"},
+	{fn_ofd_w,   fn_ofd_w,
+		NULL, NULL, "OFD write locks vs OFD   write locks"},
+
+	{fn_ofd_r,   fn_ofd_w,   fn_posix_w,
+		      NULL, "OFD   r/w locks vs POSIX write locks"},
+	{fn_posix_r, fn_ofd_w,   fn_posix_w,
+		      NULL, "POSIX r/w locks vs OFD   write locks"},
+	{fn_ofd_r,   fn_posix_r, fn_ofd_w,
+		      NULL, "OFD   r/w locks vs POSIX read locks"},
+	{fn_ofd_r,   fn_posix_r, fn_posix_w,
+		      NULL, "POSIX r/w locks vs OFD   read locks"},
+
+	{fn_ofd_r,   fn_posix_r, fn_posix_w, fn_ofd_w,
+			    "OFD   r/w locks vs POSIX r/w locks"},
+};
+
+static void tests(unsigned int i)
+{
+	test_fn(tcases[i].fn0, tcases[i].fn1,
+		tcases[i].fn2, tcases[i].fn3, tcases[i].desc);
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v6] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
> +} tcases[] = {
> +
> +	{fn_ofd_r,   fn_ofd_w,
> +		NULL, NULL, "OFD read  locks vs OFD   write locks"},
> +	{fn_ofd_w,   fn_posix_w,
> +		NULL, NULL, "OFD write locks vs POSIX write locks"},
> +	{fn_ofd_r,   fn_posix_w,
> +		NULL, NULL, "OFD read  locks vs POSIX write locks"},
> +	{fn_posix_r, fn_ofd_w,
> +		NULL, NULL, "OFD write locks vs POSIX read  locks"},
> +	{fn_ofd_w,   fn_ofd_w,
> +		NULL, NULL, "OFD write locks vs OFD   write locks"},
> +
> +	{fn_ofd_r,   fn_ofd_w,   fn_posix_w,
> +		      NULL, "OFD   r/w locks vs POSIX write locks"},
> +	{fn_posix_r, fn_ofd_w,   fn_posix_w,
> +		      NULL, "POSIX r/w locks vs OFD   write locks"},
> +	{fn_ofd_r,   fn_posix_r, fn_ofd_w,
> +		      NULL, "OFD   r/w locks vs POSIX read locks"},
> +	{fn_ofd_r,   fn_posix_r, fn_posix_w,
> +		      NULL, "POSIX r/w locks vs OFD   read locks"},
> +
> +	{fn_ofd_r,   fn_posix_r, fn_posix_w, fn_ofd_w,
> +			    "OFD   r/w locks vs POSIX r/w locks"},

The cases that concurently run posix read vs. posix write locks
unfortunately does not work, since posix locks are process based we
cannot have more than one lock on a file region per process (threads are
counted as single process as well) and conflicting locks are converted
i.e. the last two will fail with wrongly read data in the fn_posix_r
funciton in a case that posix read and posix write locks were held
concurently (which happens for me and the test fails on my testing
machine).

We can run as many threads with ofd locks as we want, since these are
based on open file descriptors and we can add exactly one posix lock
thread to the mix as well but no more than that.

Moreover I think that we may need a pthread_barrier before we close the
file descriptors in the test functions since the posix locks are dropped
when any file descriptor asociated with the file is closed, since if we
run more than two threads and ofd thread closes the fd, the posix thread
looses it's lock and the remaining ofd thread(s) run concurently, it's
unlikely that something like this would happen, but it's possible.

> +};
> +
> +static void tests(unsigned int i)
> +{
> +	test_fn(tcases[i].fn0, tcases[i].fn1,
> +		tcases[i].fn2, tcases[i].fn3, tcases[i].desc);
> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "3.15",
> +	.needs_tmpdir = 1,
> +	.test = tests,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.setup = setup
> +};
> -- 
> 1.8.3.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v7] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

Testing OFD locks racing with POSIX locks:

  OFD read  lock   vs   OFD   write lock
  OFD read  lock   vs   POSIX write lock
  OFD write lock   vs   POSIX write lock
  OFD write lock   vs   POSIX read  lock
  OFD write lock   vs   OFD   write lock
  OFD   r/w locks  vs   POSIX write locks
  OFD   r/w locks  vs   POSIX read  locks

Signed-off-by: Xiong Zhou <xzhou@redhat.com>
---

v7:
  Add pthread_barrier.
  Run only one kind of POSIX lock threads.
  Add fn_dummy, save efforts to look at thread funcs before firing them.

v6:
  Use param->length in thread_fn as r/w size instead of
write_size directly. Thanks to C99.

  Reset fail_flag after each sub case.

  Race up to 4 bunches of threads acquiring these 4 kinds of locks,
not just only 2 of them. (kind crazy)

v5:
  Optimize data checking.

v4:
  Add .gitignore entry.
  Count failures in reader, then check it in main loop.
  Other fix.

v3:
  Use tst_fill_file.
  Loop in test function.
  Other fix.

v2:
  Basically rewritten.
  Pass structure parameter to threads.
  Block on read lock.
  Spawn racing threads one by one.
  Use tcnt iteration.
 runtest/syscalls                          |   2 +
 testcases/kernel/syscalls/.gitignore      |   2 +
 testcases/kernel/syscalls/fcntl/Makefile  |   3 +
 testcases/kernel/syscalls/fcntl/fcntl36.c | 419 ++++++++++++++++++++++++++++++
 4 files changed, 426 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fcntl/fcntl36.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 407e055..f2b2cbc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -271,6 +271,8 @@ fcntl34 fcntl34
 fcntl34_64 fcntl34_64
 fcntl35 fcntl35
 fcntl35_64 fcntl35_64
+fcntl36 fcntl36
+fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 2a7026d..32193e1 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -234,6 +234,8 @@
 /fcntl/fcntl34_64
 /fcntl/fcntl35
 /fcntl/fcntl35_64
+/fcntl/fcntl36
+/fcntl/fcntl36_64
 /fdatasync/fdatasync01
 /fdatasync/fdatasync02
 /flistxattr/flistxattr01
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index d78dd72..ae37214 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -24,6 +24,9 @@ fcntl33_64: LDLIBS+=-lrt
 fcntl34: LDLIBS += -lpthread
 fcntl34_64: LDLIBS += -lpthread
 
+fcntl36: LDLIBS += -lpthread
+fcntl36_64: LDLIBS += -lpthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 include $(abs_srcdir)/../utils/newer_64.mk
 
diff --git a/testcases/kernel/syscalls/fcntl/fcntl36.c b/testcases/kernel/syscalls/fcntl/fcntl36.c
new file mode 100644
index 0000000..fad3d8a
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl36.c
@@ -0,0 +1,419 @@
+/*
+ * Copyright (c) 2017 Red Hat Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Xiong Zhou <xzhou@redhat.com>
+ *
+ * This is testing OFD locks racing with POSIX locks:
+ *
+ *	OFD read  lock   vs   OFD   write lock
+ *	OFD read  lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX write lock
+ *	OFD write lock   vs   POSIX read  lock
+ *	OFD write lock   vs   OFD   write lock
+ *
+ *	OFD   r/w locks vs POSIX write locks
+ *	OFD   r/w locks vs POSIX read locks
+ *
+ * For example:
+ *
+ * 	Init an file with preset values.
+ *
+ * 	Threads acquire OFD READ  locks to read  a 4k section start from 0;
+ * 		checking data read back, there should not be any surprise
+ * 		values and data should be consistent in a 2k block.
+ *
+ * 	Threads acquire OFD WRITE locks to write a 4k section start from 1k,
+ * 		writing different values in different threads.
+ *
+ * 	Check file data after racing, there should not be any surprise values
+ * 		and data should be consistent in a 1k block.
+ *
+ *
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <sched.h>
+#include <errno.h>
+
+#include "lapi/fcntl.h"
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int thread_cnt;
+static int fail_flag = 0;
+static volatile int loop_flag = 1;
+static const int max_thread_cnt = 32;
+static const char fname[] = "tst_ofd_posix_locks";
+static const long write_size = 4096;
+static pthread_barrier_t barrier;
+
+struct param {
+	long offset;
+	long length;
+	long cnt;
+};
+
+static void setup(void)
+{
+	thread_cnt = tst_ncpus_conf() * 3;
+	if (thread_cnt > max_thread_cnt)
+		thread_cnt = max_thread_cnt;
+}
+
+/* OFD write lock writing data*/
+static void *fn_ofd_w(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, pa->length);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	pthread_barrier_wait(&barrier);
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX write lock writing data*/
+static void *fn_posix_w(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int fd = SAFE_OPEN(fname, O_RDWR);
+	long wt = pa->cnt;
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, wt, pa->length);
+
+		lck.l_type = F_WRLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_WRITE(1, fd, buf, pa->length);
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		wt++;
+		if (wt >= 255)
+			wt = pa->cnt;
+
+		sched_yield();
+	}
+
+	pthread_barrier_wait(&barrier);
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* OFD read lock reading data*/
+static void *fn_ofd_r(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+		.l_pid    = 0,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, pa->length);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+
+			int j = (i / (pa->length/4)) * pa->length/4;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_OFD_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	pthread_barrier_wait(&barrier);
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+/* POSIX read lock reading data */
+static void *fn_posix_r(void *arg)
+{
+	struct param *pa = (struct param *)arg;
+	unsigned char buf[pa->length];
+	int i;
+	int fd = SAFE_OPEN(fname, O_RDWR);
+
+	struct flock64 lck = {
+		.l_whence = SEEK_SET,
+		.l_start  = pa->offset,
+		.l_len    = pa->length,
+	};
+
+	while (loop_flag) {
+
+		memset(buf, 0, pa->length);
+
+		lck.l_type = F_RDLCK;
+		SAFE_FCNTL(fd, F_SETLKW, &lck);
+
+		/* rlock acquired */
+		SAFE_LSEEK(fd, pa->offset, SEEK_SET);
+		SAFE_READ(1, fd, buf, pa->length);
+
+		/* Verifying data read */
+		for (i = 0; i < pa->length; i++) {
+
+			if (buf[i] < 1 || buf[i] > 254) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+
+			int j = (i / (pa->length/4)) * pa->length/4;
+
+			if (buf[i] != buf[j]) {
+
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					pa->offset + i, buf[i]);
+				fail_flag = 1;
+				break;
+			}
+		}
+
+		lck.l_type = F_UNLCK;
+		SAFE_FCNTL(fd, F_SETLK, &lck);
+
+		sched_yield();
+	}
+
+	pthread_barrier_wait(&barrier);
+	SAFE_CLOSE(fd);
+	return NULL;
+}
+
+static void *fn_dummy(void *arg)
+{
+	arg = NULL;
+
+	pthread_barrier_wait(&barrier);
+	return arg;
+}
+
+/* Test different functions and verify data */
+static void test_fn(void *f0(void *), void *f1(void *),
+		    void *f2(void *), const char *msg)
+{
+	int i, k, fd;
+	pthread_t id0[thread_cnt];
+	pthread_t id1[thread_cnt];
+	pthread_t id2[thread_cnt];
+	struct param p0[thread_cnt];
+	struct param p1[thread_cnt];
+	struct param p2[thread_cnt];
+	unsigned char buf[write_size];
+
+	tst_res(TINFO, msg);
+
+	if (tst_fill_file(fname, 1, write_size, thread_cnt + 1))
+		tst_brk(TBROK, "Failed to create tst file");
+
+	if (pthread_barrier_init(&barrier, NULL, thread_cnt*3) != 0)
+		tst_brk(TBROK, "Failed to init pthread barrier");
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		p0[i].offset = i * write_size;
+		p0[i].length = write_size;
+		p0[i].cnt = i + 2;
+
+		p1[i].offset = i * write_size + write_size / 4;
+		p1[i].length = write_size;
+		p1[i].cnt = i + 2;
+
+		p2[i].offset = i * write_size + write_size / 2;
+		p2[i].length = write_size;
+		p2[i].cnt = i + 2;
+	}
+
+	fail_flag = 0;
+	loop_flag = 1;
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		SAFE_PTHREAD_CREATE(id0 + i, NULL, f0, (void *)&p0[i]);
+		SAFE_PTHREAD_CREATE(id1 + i, NULL, f1, (void *)&p1[i]);
+		SAFE_PTHREAD_CREATE(id2 + i, NULL, f2, (void *)&p2[i]);
+	}
+
+	sleep(1);
+	loop_flag = 0;
+
+	for (i = 0; i < thread_cnt; i++) {
+
+		SAFE_PTHREAD_JOIN(id0[i], NULL);
+		SAFE_PTHREAD_JOIN(id1[i], NULL);
+		SAFE_PTHREAD_JOIN(id2[i], NULL);
+	}
+
+	fd = SAFE_OPEN(fname, O_RDONLY);
+
+	for (i = 0; i < thread_cnt * 4; i++) {
+
+		SAFE_READ(1, fd, buf, write_size/4);
+
+		for (k = 0; k < write_size/4; k++) {
+
+			if (buf[k] < 2 || buf[k] > 254) {
+
+				if (i < 3 && buf[k] == 1)
+					continue;
+				tst_res(TFAIL, "Unexpected data "
+					"offset %ld value %d",
+					i * write_size / 4 + k, buf[k]);
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+
+		for (k = 1; k < write_size/4; k++) {
+
+			if (buf[k] != buf[0]) {
+				tst_res(TFAIL, "Unexpected block read");
+				SAFE_CLOSE(fd);
+				return;
+			}
+		}
+	}
+
+	if (pthread_barrier_destroy(&barrier) != 0)
+		tst_brk(TBROK, "Failed to destroy pthread barrier");
+
+	SAFE_CLOSE(fd);
+	if (fail_flag == 0)
+		tst_res(TPASS, "Access between threads synchronized");
+}
+
+static struct tcase {
+
+	void *(*fn0)(void *);
+	void *(*fn1)(void *);
+	void *(*fn2)(void *);
+	const char *desc;
+
+} tcases[] = {
+
+	{fn_ofd_r, fn_ofd_w, fn_dummy, "OFD read lock vs OFD write lock"},
+
+	{fn_ofd_w, fn_posix_w, fn_dummy, "OFD write lock vs POSIX write lock"},
+
+	{fn_ofd_r, fn_posix_w, fn_dummy, "OFD read lock vs POSIX write lock"},
+
+	{fn_ofd_w, fn_posix_r, fn_dummy, "OFD write lock vs POSIX read lock"},
+
+	{fn_ofd_w, fn_ofd_w, fn_dummy, "OFD write lock vs OFD write lock"},
+
+	{fn_ofd_r, fn_ofd_w, fn_posix_w, "OFD r/w lock vs POSIX write lock"},
+
+	{fn_ofd_r, fn_ofd_w, fn_posix_r, "OFD r/w lock vs POSIX read lock"},
+};
+
+static void tests(unsigned int i)
+{
+	test_fn(tcases[i].fn0, tcases[i].fn1, tcases[i].fn2, tcases[i].desc);
+}
+
+static struct tst_test test = {
+	.min_kver = "3.15",
+	.needs_tmpdir = 1,
+	.test = tests,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup
+};
-- 
1.8.3.1

[LTP] [PATCH v7] syscalls/fcntl36: add tests for OFD locks

[Cyril Hrubis]

Hi!
Pushed with minor changes, mostly whitespace related, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v7] syscalls/fcntl36: add tests for OFD locks

[Xiong Zhou]

On Tue, Sep 05, 2017 at 02:06:29PM +0200, Cyril Hrubis wrote:
> Hi!
> Pushed with minor changes, mostly whitespace related, thanks.

Thanks for reviewing!

> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz