[PATCH V2 0/5] xfsprogs: generic serialisation primitives

[PATCH V2 0/5] xfsprogs: generic serialisation primitives

[Chandan Babu R]

This patchset provides userspace equivalents for spinlocks, atomics,
completion and semaphores that the shared libxfs code depends on in
the kernel.

I have executed fstests on a 4k block sized filesystem which has
reflink and rmap enabled.

Changelog:
V1 -> V2:
1. Provide m4 macros to detect availability of liburcu on the system
   where xfsprogs is being built.
2. Initialize inode log item's spin lock.
3. Swap order of arguments provided to atomic[64]_[add|sub]().

Dave Chinner (5):
  xfsprogs: introduce liburcu support
  libxfs: add spinlock_t wrapper
  atomic: convert to uatomic
  libxfs: add kernel-compatible completion API
  libxfs: add wrappers for kernel semaphores

 configure.ac               |  3 ++
 copy/Makefile              |  3 +-
 copy/xfs_copy.c            |  3 ++
 db/Makefile                |  3 +-
 debian/control             |  2 +-
 growfs/Makefile            |  3 +-
 include/Makefile           |  3 ++
 include/atomic.h           | 65 +++++++++++++++++++++++++++++++-------
 include/builddefs.in       |  4 ++-
 include/completion.h       | 61 +++++++++++++++++++++++++++++++++++
 include/libxfs.h           |  3 ++
 include/platform_defs.h.in |  1 +
 include/sema.h             | 35 ++++++++++++++++++++
 include/spinlock.h         | 25 +++++++++++++++
 include/xfs_inode.h        |  1 +
 include/xfs_mount.h        |  2 ++
 include/xfs_trans.h        |  1 +
 libfrog/workqueue.c        |  3 ++
 libxfs/init.c              |  7 +++-
 libxfs/libxfs_priv.h       |  9 +++---
 libxfs/logitem.c           |  4 ++-
 libxfs/rdwr.c              |  2 ++
 logprint/Makefile          |  3 +-
 m4/Makefile                |  1 +
 m4/package_urcu.m4         | 24 ++++++++++++++
 mdrestore/Makefile         |  3 +-
 mkfs/Makefile              |  2 +-
 repair/Makefile            |  2 +-
 repair/prefetch.c          |  9 ++++--
 repair/progress.c          |  4 ++-
 scrub/Makefile             |  3 +-
 scrub/progress.c           |  2 ++
 32 files changed, 265 insertions(+), 31 deletions(-)
 create mode 100644 include/completion.h
 create mode 100644 include/sema.h
 create mode 100644 include/spinlock.h
 create mode 100644 m4/package_urcu.m4

-- 
2.30.2

[PATCH V2 1/5] xfsprogs: introduce liburcu support

[Chandan Babu R]

From: Dave Chinner <dchinner@redhat.com>

The upcoming buffer cache rework/kerenl sync-up requires atomic
variables. I could use C++11 atomics build into GCC, but they are a
pain to work with and shoe-horn into the kernel atomic variable API.

Much easier is to introduce a dependency on liburcu - the userspace
RCU library. This provides atomic variables that very closely match
the kernel atomic variable API, and it provides a very similar
memory model and memory barrier support to the kernel. And we get
RCU support that has an identical interface to the kernel and works
the same way.

Hence kernel code written with RCU algorithms and atomic variables
will just slot straight into the userspace xfsprogs code without us
having to think about whether the lockless algorithms will work in
userspace or not. This reduces glue and hoop jumping, and gets us
a step closer to having the entire userspace libxfs code MT safe.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
[chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]
Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
---
 configure.ac               |  3 +++
 copy/Makefile              |  3 ++-
 copy/xfs_copy.c            |  3 +++
 db/Makefile                |  3 ++-
 debian/control             |  2 +-
 growfs/Makefile            |  3 ++-
 include/builddefs.in       |  4 +++-
 include/platform_defs.h.in |  1 +
 libfrog/workqueue.c        |  3 +++
 libxfs/init.c              |  3 +++
 libxfs/libxfs_priv.h       |  3 +--
 logprint/Makefile          |  3 ++-
 m4/Makefile                |  1 +
 m4/package_urcu.m4         | 24 ++++++++++++++++++++++++
 mdrestore/Makefile         |  3 ++-
 mkfs/Makefile              |  2 +-
 repair/Makefile            |  2 +-
 repair/prefetch.c          |  9 +++++++--
 repair/progress.c          |  4 +++-
 scrub/Makefile             |  3 ++-
 scrub/progress.c           |  2 ++
 21 files changed, 69 insertions(+), 15 deletions(-)
 create mode 100644 m4/package_urcu.m4

diff --git a/configure.ac b/configure.ac
index 56871745..61c6cf40 100644
--- a/configure.ac
+++ b/configure.ac
@@ -154,6 +154,9 @@ AC_PACKAGE_NEED_UUIDCOMPARE
 AC_PACKAGE_NEED_PTHREAD_H
 AC_PACKAGE_NEED_PTHREADMUTEXINIT
 
+AC_PACKAGE_NEED_URCU_H
+AC_PACKAGE_NEED_RCU_SET_POINTER_SYM
+
 AC_HAVE_FADVISE
 AC_HAVE_MADVISE
 AC_HAVE_MINCORE
diff --git a/copy/Makefile b/copy/Makefile
index 449b235f..1b00cd0d 100644
--- a/copy/Makefile
+++ b/copy/Makefile
@@ -9,7 +9,8 @@ LTCOMMAND = xfs_copy
 CFILES = xfs_copy.c
 HFILES = xfs_copy.h
 
-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBPTHREAD) $(LIBRT)
+LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBPTHREAD) $(LIBRT) \
+	  $(LIBURCU)
 LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index fc7d225f..f5eff969 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -110,6 +110,7 @@ do_message(int flags, int code, const char *fmt, ...)
 		fprintf(stderr,
 			_("Aborting XFS copy -- logfile error -- reason: %s\n"),
 			strerror(errno));
+		rcu_unregister_thread();
 		pthread_exit(NULL);
 	}
 }
@@ -224,6 +225,7 @@ begin_reader(void *arg)
 {
 	thread_args	*args = arg;
 
+	rcu_register_thread();
 	for (;;) {
 		pthread_mutex_lock(&args->wait);
 		if (do_write(args, NULL))
@@ -243,6 +245,7 @@ handle_error:
 	if (--glob_masks.num_working == 0)
 		pthread_mutex_unlock(&mainwait);
 	pthread_mutex_unlock(&glob_masks.mutex);
+	rcu_unregister_thread();
 	pthread_exit(NULL);
 	return NULL;
 }
diff --git a/db/Makefile b/db/Makefile
index beafb105..5c017898 100644
--- a/db/Makefile
+++ b/db/Makefile
@@ -18,7 +18,8 @@ CFILES = $(HFILES:.h=.c) btdump.c btheight.c convert.c info.c namei.c \
 	timelimit.c
 LSRCFILES = xfs_admin.sh xfs_ncheck.sh xfs_metadump.sh
 
-LLDLIBS	= $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
+LLDLIBS	= $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) \
+	  $(LIBURCU)
 LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBFROG)
 LLDFLAGS += -static-libtool-libs
 
diff --git a/debian/control b/debian/control
index e4ec897c..71c08167 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>, Bastian Germann <bastiangermann@fishpost.de>
-Build-Depends: libinih-dev (>= 53), uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, pkg-config
+Build-Depends: libinih-dev (>= 53), uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, pkg-config, liburcu-dev
 Standards-Version: 4.0.0
 Homepage: https://xfs.wiki.kernel.org/
 
diff --git a/growfs/Makefile b/growfs/Makefile
index a107d348..08601de7 100644
--- a/growfs/Makefile
+++ b/growfs/Makefile
@@ -9,7 +9,8 @@ LTCOMMAND = xfs_growfs
 
 CFILES = xfs_growfs.c
 
-LLDLIBS = $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
+LLDLIBS = $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) \
+	  $(LIBURCU)
 
 ifeq ($(ENABLE_EDITLINE),yes)
 LLDLIBS += $(LIBEDITLINE) $(LIBTERMCAP)
diff --git a/include/builddefs.in b/include/builddefs.in
index e8f447f9..78eddf4a 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -22,6 +22,7 @@ LDFLAGS =
 
 LIBRT = @librt@
 LIBUUID = @libuuid@
+LIBURCU = @liburcu@
 LIBPTHREAD = @libpthread@
 LIBTERMCAP = @libtermcap@
 LIBEDITLINE = @libeditline@
@@ -125,7 +126,8 @@ CROND_DIR = @crond_dir@
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
 
-PCFLAGS = -D_GNU_SOURCE $(GCCFLAGS)
+# _LGPL_SOURCE is for liburcu to work correctly with GPL/LGPL programs
+PCFLAGS = -D_LGPL_SOURCE -D_GNU_SOURCE $(GCCFLAGS)
 ifeq ($(HAVE_UMODE_T),yes)
 PCFLAGS += -DHAVE_UMODE_T
 endif
diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index 539bdbec..7c6b3ada 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -23,6 +23,7 @@
 #include <limits.h>
 #include <stdbool.h>
 #include <libgen.h>
+#include <urcu.h>
 
 typedef struct filldir		filldir_t;
 
diff --git a/libfrog/workqueue.c b/libfrog/workqueue.c
index 8c1a163e..702a53e2 100644
--- a/libfrog/workqueue.c
+++ b/libfrog/workqueue.c
@@ -11,6 +11,7 @@
 #include <stdbool.h>
 #include <errno.h>
 #include <assert.h>
+#include <urcu.h>
 #include "workqueue.h"
 
 /* Main processing thread */
@@ -24,6 +25,7 @@ workqueue_thread(void *arg)
 	 * Loop pulling work from the passed in work queue.
 	 * Check for notification to exit after every chunk of work.
 	 */
+	rcu_register_thread();
 	while (1) {
 		pthread_mutex_lock(&wq->lock);
 
@@ -60,6 +62,7 @@ workqueue_thread(void *arg)
 		(wi->function)(wi->queue, wi->index, wi->arg);
 		free(wi);
 	}
+	rcu_unregister_thread();
 
 	return NULL;
 }
diff --git a/libxfs/init.c b/libxfs/init.c
index 1ec83791..b06faf8a 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -310,6 +310,8 @@ libxfs_init(libxfs_init_t *a)
 	fd = -1;
 	flags = (a->isreadonly | a->isdirect);
 
+	rcu_init();
+	rcu_register_thread();
 	radix_tree_init();
 
 	if (a->volname) {
@@ -1023,6 +1025,7 @@ libxfs_destroy(
 	libxfs_bcache_free();
 	cache_destroy(libxfs_bcache);
 	leaked = destroy_zones();
+	rcu_unregister_thread();
 	if (getenv("LIBXFS_LEAK_CHECK") && leaked)
 		exit(1);
 }
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 7181a858..db90e173 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -210,8 +210,7 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #define spin_unlock(a)		((void) 0)
 #define likely(x)		(x)
 #define unlikely(x)		(x)
-#define rcu_read_lock()		((void) 0)
-#define rcu_read_unlock()	((void) 0)
+
 /* Need to be able to handle this bare or in control flow */
 static inline bool WARN_ON(bool expr) {
 	return (expr);
diff --git a/logprint/Makefile b/logprint/Makefile
index 758504b3..cdedbd0d 100644
--- a/logprint/Makefile
+++ b/logprint/Makefile
@@ -12,7 +12,8 @@ CFILES = logprint.c \
 	 log_copy.c log_dump.c log_misc.c \
 	 log_print_all.c log_print_trans.c log_redo.c
 
-LLDLIBS	= $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
+LLDLIBS	= $(LIBXFS) $(LIBXLOG) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) \
+	  $(LIBURCU)
 LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/m4/Makefile b/m4/Makefile
index c6c73dc9..73120530 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -24,6 +24,7 @@ LSRCFILES = \
 	package_services.m4 \
 	package_types.m4 \
 	package_icu.m4 \
+	package_urcu.m4 \
 	package_utilies.m4 \
 	package_uuiddev.m4 \
 	multilib.m4 \
diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
new file mode 100644
index 00000000..74a24db9
--- /dev/null
+++ b/m4/package_urcu.m4
@@ -0,0 +1,24 @@
+AC_DEFUN([AC_PACKAGE_NEED_URCU_H],
+  [ AC_CHECK_HEADERS([urcu.h])
+    if test $ac_cv_header_urcu_h = no; then
+	echo
+	echo 'FATAL ERROR: could not find a valid URCU header.'
+	echo 'Install the Userspace RCU development package.'
+	exit 1
+    fi
+  ])
+
+AC_DEFUN([AC_PACKAGE_NEED_RCU_SET_POINTER_SYM],
+  [ AC_CHECK_FUNCS(rcu_set_pointer_sym)
+    if test $ac_cv_func_rcu_set_pointer_sym = yes; then
+	liburcu=""
+    else
+	AC_CHECK_LIB(urcu, rcu_set_pointer_sym,, [
+	    echo
+	    echo 'FATAL ERROR: could not find a valid urcu library.'
+	    echo 'Install the Userspace RCU development package.'
+	    exit 1])
+	liburcu="-lurcu"
+    fi
+    AC_SUBST(liburcu)
+  ])
diff --git a/mdrestore/Makefile b/mdrestore/Makefile
index d946955b..8f28ddab 100644
--- a/mdrestore/Makefile
+++ b/mdrestore/Makefile
@@ -8,7 +8,8 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = xfs_mdrestore
 CFILES = xfs_mdrestore.c
 
-LLDLIBS = $(LIBXFS) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBUUID)
+LLDLIBS = $(LIBXFS) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBUUID) \
+	  $(LIBURCU)
 LTDEPENDENCIES = $(LIBXFS) $(LIBFROG)
 LLDFLAGS = -static
 
diff --git a/mkfs/Makefile b/mkfs/Makefile
index b8805f7e..811ba9db 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -11,7 +11,7 @@ HFILES =
 CFILES = proto.c xfs_mkfs.c
 
 LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
-	$(LIBUUID) $(LIBINIH)
+	$(LIBUUID) $(LIBINIH) $(LIBURCU)
 LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/repair/Makefile b/repair/Makefile
index 5f0764d1..47536ca1 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -72,7 +72,7 @@ CFILES = \
 	xfs_repair.c
 
 LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) \
-	$(LIBPTHREAD) $(LIBBLKID)
+	$(LIBPTHREAD) $(LIBBLKID) $(LIBURCU)
 LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 48affa18..22a0c0c9 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -660,6 +660,7 @@ pf_io_worker(
 	if (buf == NULL)
 		return NULL;
 
+	rcu_register_thread();
 	pthread_mutex_lock(&args->lock);
 	while (!args->queuing_done || !btree_is_empty(args->io_queue)) {
 		pftrace("waiting to start prefetch I/O for AG %d", args->agno);
@@ -682,6 +683,7 @@ pf_io_worker(
 	free(buf);
 
 	pftrace("finished prefetch I/O for AG %d", args->agno);
+	rcu_unregister_thread();
 
 	return NULL;
 }
@@ -726,6 +728,8 @@ pf_queuing_worker(
 	struct xfs_ino_geometry	*igeo = M_IGEO(mp);
 	unsigned long long	cluster_mask;
 
+	rcu_register_thread();
+
 	cluster_mask = (1ULL << igeo->inodes_per_cluster) - 1;
 
 	for (i = 0; i < PF_THREAD_COUNT; i++) {
@@ -739,7 +743,7 @@ pf_queuing_worker(
 			args->io_threads[i] = 0;
 			if (i == 0) {
 				pf_skip_prefetch_thread(args);
-				return NULL;
+				goto out;
 			}
 			/*
 			 * since we have at least one I/O thread, use them for
@@ -779,7 +783,6 @@ pf_queuing_worker(
 			 * Start processing as well, in case everything so
 			 * far was already prefetched and the queue is empty.
 			 */
-			
 			pf_start_io_workers(args);
 			pf_start_processing(args);
 			sem_wait(&args->ra_count);
@@ -841,6 +844,8 @@ pf_queuing_worker(
 	if (next_args)
 		pf_create_prefetch_thread(next_args);
 
+out:
+	rcu_unregister_thread();
 	return NULL;
 }
 
diff --git a/repair/progress.c b/repair/progress.c
index e5a9c1ef..f6c4d988 100644
--- a/repair/progress.c
+++ b/repair/progress.c
@@ -182,6 +182,7 @@ progress_rpt_thread (void *p)
 		do_error (_("progress_rpt: cannot malloc progress msg buffer\n"));
 
 	running = 1;
+	rcu_register_thread();
 
 	/*
 	 * Specify a repeating timer that fires each MSG_INTERVAL seconds.
@@ -286,7 +287,8 @@ progress_rpt_thread (void *p)
 		do_warn(_("cannot delete timer\n"));
 
 	free (msgbuf);
-	return (NULL);
+	rcu_unregister_thread();
+	return NULL;
 }
 
 int
diff --git a/scrub/Makefile b/scrub/Makefile
index 47c887eb..849e3afd 100644
--- a/scrub/Makefile
+++ b/scrub/Makefile
@@ -71,7 +71,8 @@ spacemap.c \
 vfs.c \
 xfs_scrub.c
 
-LLDLIBS += $(LIBHANDLE) $(LIBFROG) $(LIBPTHREAD) $(LIBICU_LIBS) $(LIBRT)
+LLDLIBS += $(LIBHANDLE) $(LIBFROG) $(LIBPTHREAD) $(LIBICU_LIBS) $(LIBRT) \
+	$(LIBURCU)
 LTDEPENDENCIES += $(LIBHANDLE) $(LIBFROG)
 LLDFLAGS = -static
 
diff --git a/scrub/progress.c b/scrub/progress.c
index 15247b7c..a3d096f9 100644
--- a/scrub/progress.c
+++ b/scrub/progress.c
@@ -116,6 +116,7 @@ progress_report_thread(void *arg)
 	struct timespec		abstime;
 	int			ret;
 
+	rcu_register_thread();
 	pthread_mutex_lock(&pt.lock);
 	while (1) {
 		uint64_t	progress_val;
@@ -139,6 +140,7 @@ progress_report_thread(void *arg)
 			progress_report(progress_val);
 	}
 	pthread_mutex_unlock(&pt.lock);
+	rcu_unregister_thread();
 	return NULL;
 }
 
-- 
2.30.2

[PATCH V2 2/5] libxfs: add spinlock_t wrapper

[Chandan Babu R]

From: Dave Chinner <dchinner@redhat.com>

These provide the kernel spinlock_t interface, but are *not*
spinlocks. Spinlocks cannot be used by general purpose userspace
processes due to the fact they cannot control task preemption and
scheduling reliability. Hence these are implemented as a
pthread_mutex_t, similar to the way the kernel RT build implements
spinlock_t as a kernel mutex.

Because the current libxfs spinlock "implementation" just makes
spinlocks go away, we have to also add initialisation to spinlocks
that libxfs uses that are missing from the userspace implementation.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
[chandan.babu@oracle.com: Initialize inode log item spin lock]
Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
---
 include/Makefile     |  1 +
 include/libxfs.h     |  1 +
 include/spinlock.h   | 25 +++++++++++++++++++++++++
 include/xfs_inode.h  |  1 +
 include/xfs_mount.h  |  2 ++
 include/xfs_trans.h  |  1 +
 libxfs/init.c        |  4 +++-
 libxfs/libxfs_priv.h |  4 +---
 libxfs/logitem.c     |  4 +++-
 libxfs/rdwr.c        |  2 ++
 10 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 include/spinlock.h

diff --git a/include/Makefile b/include/Makefile
index 632b819f..f7c40a5c 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -16,6 +16,7 @@ LIBHFILES = libxfs.h \
 	kmem.h \
 	list.h \
 	parent.h \
+	spinlock.h \
 	xfs_inode.h \
 	xfs_log_recover.h \
 	xfs_metadump.h \
diff --git a/include/libxfs.h b/include/libxfs.h
index bc07655e..a494a1d4 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -18,6 +18,7 @@
 #include "kmem.h"
 #include "libfrog/radix-tree.h"
 #include "atomic.h"
+#include "spinlock.h"
 
 #include "xfs_types.h"
 #include "xfs_fs.h"
diff --git a/include/spinlock.h b/include/spinlock.h
new file mode 100644
index 00000000..8da2325c
--- /dev/null
+++ b/include/spinlock.h
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019-20 RedHat, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __LIBXFS_SPINLOCK_H__
+#define __LIBXFS_SPINLOCK_H__
+
+/*
+ * This implements kernel compatible spinlock exclusion semantics. These,
+ * however, are not spinlocks, as spinlocks cannot be reliably implemented in
+ * userspace without using realtime scheduling task contexts. Hence this
+ * interface is implemented with pthread mutexes and so can block, but this is
+ * no different to the kernel RT build which replaces spinlocks with mutexes.
+ * Hence we know it works.
+ */
+
+typedef pthread_mutex_t	spinlock_t;
+
+#define spin_lock_init(l)	pthread_mutex_init(l, NULL)
+#define spin_lock(l)           pthread_mutex_lock(l)
+#define spin_trylock(l)        (pthread_mutex_trylock(l) != EBUSY)
+#define spin_unlock(l)         pthread_mutex_unlock(l)
+
+#endif /* __LIBXFS_SPINLOCK_H__ */
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 0551fe45..08a62d83 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -43,6 +43,7 @@ struct inode {
 	struct timespec64	i_atime;
 	struct timespec64	i_mtime;
 	struct timespec64	i_ctime;
+	spinlock_t		i_lock;
 };
 
 static inline uint32_t i_uid_read(struct inode *inode)
diff --git a/include/xfs_mount.h b/include/xfs_mount.h
index 12019c4b..2f320880 100644
--- a/include/xfs_mount.h
+++ b/include/xfs_mount.h
@@ -22,6 +22,7 @@ typedef struct xfs_mount {
 #define m_icount	m_sb.sb_icount
 #define m_ifree		m_sb.sb_ifree
 #define m_fdblocks	m_sb.sb_fdblocks
+	spinlock_t		m_sb_lock;
 
 	/*
 	 * Bitsets of per-fs metadata that have been checked and/or are sick.
@@ -32,6 +33,7 @@ typedef struct xfs_mount {
 
 	char			*m_fsname;	/* filesystem name */
 	int			m_bsize;	/* fs logical block size */
+	spinlock_t		m_agirotor_lock;
 	xfs_agnumber_t		m_agfrotor;	/* last ag where space found */
 	xfs_agnumber_t		m_agirotor;	/* last ag dir inode alloced */
 	xfs_agnumber_t		m_maxagi;	/* highest inode alloc group */
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index ad76ecfd..2c55bb85 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -35,6 +35,7 @@ struct xfs_inode_log_item {
 	unsigned int		ili_last_fields;	/* fields when flushed*/
 	unsigned int		ili_fields;		/* fields to be logged */
 	unsigned int		ili_fsync_fields;	/* ignored by userspace */
+	spinlock_t		ili_lock;
 };
 
 typedef struct xfs_buf_log_item {
diff --git a/libxfs/init.c b/libxfs/init.c
index b06faf8a..2c54546b 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -743,7 +743,9 @@ libxfs_mount(
 	mp->m_flags = (LIBXFS_MOUNT_32BITINODES|LIBXFS_MOUNT_32BITINOOPT);
 	mp->m_sb = *sb;
 	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_KERNEL);
-	sbp = &(mp->m_sb);
+	sbp = &mp->m_sb;
+	spin_lock_init(&mp->m_sb_lock);
+	spin_lock_init(&mp->m_agirotor_lock);
 
 	xfs_sb_mount_common(mp, sb);
 
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index db90e173..e1e90268 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -48,6 +48,7 @@
 #include "kmem.h"
 #include "libfrog/radix-tree.h"
 #include "atomic.h"
+#include "spinlock.h"
 
 #include "xfs_types.h"
 #include "xfs_arch.h"
@@ -205,9 +206,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #endif
 
 /* miscellaneous kernel routines not in user space */
-#define spin_lock_init(a)	((void) 0)
-#define spin_lock(a)		((void) 0)
-#define spin_unlock(a)		((void) 0)
 #define likely(x)		(x)
 #define unlikely(x)		(x)
 
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4d4e8080..b073cdb4 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -144,6 +144,8 @@ xfs_inode_item_init(
 		ip->i_ino, iip);
 #endif
 
-	xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
+	spin_lock_init(&iip->ili_lock);
+
+        xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
 	iip->ili_inode = ip;
 }
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 713ef9af..a5fd0596 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1070,6 +1070,8 @@ libxfs_iget(
 	VFS_I(ip)->i_count = 1;
 	ip->i_ino = ino;
 	ip->i_mount = mp;
+	spin_lock_init(&VFS_I(ip)->i_lock);
+
 	error = xfs_imap(mp, tp, ip->i_ino, &ip->i_imap, 0);
 	if (error)
 		goto out_destroy;
-- 
2.30.2

[PATCH V2 3/5] atomic: convert to uatomic

[Chandan Babu R]

From: Dave Chinner <dchinner@redhat.com>

Now we have liburcu, we can make use of it's atomic variable
implementation. It is almost identical to the kernel API - it's just
got a "uatomic" prefix. liburcu also provides all the same aomtic
variable memory barriers as the kernel, so if we pull memory barrier
dependent kernel code across, it will just work with the right
barrier wrappers.

This is preparation the addition of more extensive atomic operations
the that kernel buffer cache requires to function correctly.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
[chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
---
 include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/include/atomic.h b/include/atomic.h
index e0e1ba84..99cb85d3 100644
--- a/include/atomic.h
+++ b/include/atomic.h
@@ -7,21 +7,64 @@
 #define __ATOMIC_H__
 
 /*
- * Warning: These are not really atomic at all. They are wrappers around the
- * kernel atomic variable interface. If we do need these variables to be atomic
- * (due to multithreading of the code that uses them) we need to add some
- * pthreads magic here.
+ * Atomics are provided by liburcu.
+ *
+ * API and guidelines for which operations provide memory barriers is here:
+ *
+ * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
+ *
+ * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.
  */
+#include <urcu/uatomic.h>
+#include "spinlock.h"
+
 typedef	int32_t	atomic_t;
 typedef	int64_t	atomic64_t;
 
-#define atomic_inc_return(x)	(++(*(x)))
-#define atomic_dec_return(x)	(--(*(x)))
+#define atomic_read(a)		uatomic_read(a)
+#define atomic_set(a, v)	uatomic_set(a, v)
+#define atomic_add(v, a)	uatomic_add(a, v)
+#define atomic_sub(v, a)	uatomic_sub(a, v)
+#define atomic_inc(a)		uatomic_inc(a)
+#define atomic_dec(a)		uatomic_dec(a)
+#define atomic_inc_return(a)	uatomic_add_return(a, 1)
+#define atomic_dec_return(a)	uatomic_sub_return(a, 1)
+#define atomic_dec_and_test(a)	(atomic_dec_return(a) == 0)
+#define cmpxchg(a, o, n)        uatomic_cmpxchg(a, o, n);
+
+static inline bool atomic_add_unless(atomic_t *a, int v, int u)
+{
+	int r = atomic_read(a);
+	int n, o;
+
+	do {
+		o = r;
+		if (o == u)
+			break;
+		n = o + v;
+		r = uatomic_cmpxchg(a, o, n);
+	} while (r != o);
+
+	return o != u;
+}
+
+static inline bool atomic_dec_and_lock(atomic_t *a, spinlock_t *lock)
+{
+	if (atomic_add_unless(a, -1, 1))
+		return 0;
+
+	spin_lock(lock);
+	if (atomic_dec_and_test(a))
+		return 1;
+	spin_unlock(lock);
+	return 0;
+}
 
-#define atomic64_read(x)	*(x)
-#define atomic64_set(x, v)	(*(x) = v)
-#define atomic64_add(v, x)	(*(x) += v)
-#define atomic64_inc(x)		((*(x))++)
-#define atomic64_dec(x)		((*(x))--)
+#define atomic64_read(x)	uatomic_read(x)
+#define atomic64_set(x, v)	uatomic_set(x, v)
+#define atomic64_add(v, a)	uatomic_add(a, v)
+#define atomic64_sub(v, a)	uatomic_sub(a, v)
+#define atomic64_inc(a)		uatomic_inc(a)
+#define atomic64_dec(a)		uatomic_dec(a)
 
 #endif /* __ATOMIC_H__ */
-- 
2.30.2

[PATCH V2 4/5] libxfs: add kernel-compatible completion API

[Chandan Babu R]

From: Dave Chinner <dchinner@redhat.com>

This is needed for the kernel buffer cache conversion to be able
to wait on IO synchrnously. It is implemented with pthread mutexes
and conditional variables.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 include/Makefile     |  1 +
 include/completion.h | 61 ++++++++++++++++++++++++++++++++++++++++++++
 include/libxfs.h     |  1 +
 libxfs/libxfs_priv.h |  1 +
 4 files changed, 64 insertions(+)
 create mode 100644 include/completion.h

diff --git a/include/Makefile b/include/Makefile
index f7c40a5c..98031e70 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -12,6 +12,7 @@ LIBHFILES = libxfs.h \
 	atomic.h \
 	bitops.h \
 	cache.h \
+	completion.h \
 	hlist.h \
 	kmem.h \
 	list.h \
diff --git a/include/completion.h b/include/completion.h
new file mode 100644
index 00000000..92194c3f
--- /dev/null
+++ b/include/completion.h
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 RedHat, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __LIBXFS_COMPLETION_H__
+#define __LIBXFS_COMPLETION_H__
+
+/*
+ * This implements kernel compatible completion semantics. This is slightly
+ * different to the way pthread conditional variables work in that completions
+ * can be signalled before the waiter tries to wait on the variable. In the
+ * pthread case, the completion is ignored and the waiter goes to sleep, whilst
+ * the kernel will see that the completion has already been completed and so
+ * will not block. This is handled through the addition of the the @signalled
+ * flag in the struct completion.
+ */
+struct completion {
+	pthread_mutex_t		lock;
+	pthread_cond_t		cond;
+	bool			signalled; /* for kernel completion behaviour */
+	int			waiters;
+};
+
+static inline void
+init_completion(struct completion *w)
+{
+	pthread_mutex_init(&w->lock, NULL);
+	pthread_cond_init(&w->cond, NULL);
+	w->signalled = false;
+}
+
+static inline void
+complete(struct completion *w)
+{
+	pthread_mutex_lock(&w->lock);
+	w->signalled = true;
+	pthread_cond_broadcast(&w->cond);
+	pthread_mutex_unlock(&w->lock);
+}
+
+/*
+ * Support for mulitple waiters requires that we count the number of waiters
+ * we have and only clear the signalled variable once all those waiters have
+ * been woken.
+ */
+static inline void
+wait_for_completion(struct completion *w)
+{
+	pthread_mutex_lock(&w->lock);
+	if (!w->signalled) {
+		w->waiters++;
+		pthread_cond_wait(&w->cond, &w->lock);
+		w->waiters--;
+	}
+	if (!w->waiters)
+		w->signalled = false;
+	pthread_mutex_unlock(&w->lock);
+}
+
+#endif /* __LIBXFS_COMPLETION_H__ */
diff --git a/include/libxfs.h b/include/libxfs.h
index a494a1d4..61475347 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -19,6 +19,7 @@
 #include "libfrog/radix-tree.h"
 #include "atomic.h"
 #include "spinlock.h"
+#include "completion.h"
 
 #include "xfs_types.h"
 #include "xfs_fs.h"
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index e1e90268..9f28fd90 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -49,6 +49,7 @@
 #include "libfrog/radix-tree.h"
 #include "atomic.h"
 #include "spinlock.h"
+#include "completion.h"
 
 #include "xfs_types.h"
 #include "xfs_arch.h"
-- 
2.30.2

[PATCH V2 5/5] libxfs: add wrappers for kernel semaphores

[Chandan Babu R]

From: Dave Chinner <dchinner@redhat.com>

Implemented via pthread mutexes.

On Linux, fast pthread mutexes don't actaully check which thread
owns the lock on unlock, so can be used in situations where the
unlock occurs in a different thread to the lock. This is
non-portable behaviour, so if other platforms are supported, this
may need to be converted to posix semaphores.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 include/Makefile     |  1 +
 include/libxfs.h     |  1 +
 include/sema.h       | 35 +++++++++++++++++++++++++++++++++++
 libxfs/libxfs_priv.h |  1 +
 4 files changed, 38 insertions(+)
 create mode 100644 include/sema.h

diff --git a/include/Makefile b/include/Makefile
index 98031e70..ce89d023 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -17,6 +17,7 @@ LIBHFILES = libxfs.h \
 	kmem.h \
 	list.h \
 	parent.h \
+	sema.h \
 	spinlock.h \
 	xfs_inode.h \
 	xfs_log_recover.h \
diff --git a/include/libxfs.h b/include/libxfs.h
index 61475347..ca5a21b0 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -20,6 +20,7 @@
 #include "atomic.h"
 #include "spinlock.h"
 #include "completion.h"
+#include "sema.h"
 
 #include "xfs_types.h"
 #include "xfs_fs.h"
diff --git a/include/sema.h b/include/sema.h
new file mode 100644
index 00000000..bcccb156
--- /dev/null
+++ b/include/sema.h
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019-20 RedHat, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __LIBXFS_SEMA_H__
+#define __LIBXFS_SEMA_H__
+
+/*
+ * This implements kernel compatible semaphore _exclusion_ semantics. It does
+ * not implement counting semaphore behaviour.
+ *
+ * This makes use of the fact that fast pthread mutexes on Linux don't check
+ * that the unlocker is the same thread that locked the mutex, and hence can be
+ * unlocked in a different thread safely.
+ *
+ * If this needs to be portable or we require counting semaphore behaviour in
+ * libxfs code, this requires re-implementation based on posix semaphores.
+ */
+struct semaphore {
+	pthread_mutex_t		lock;
+};
+
+#define sema_init(l, nolock)		\
+do {					\
+	pthread_mutex_init(&(l)->lock, NULL);	\
+	if (!nolock)			\
+		pthread_mutex_lock(&(l)->lock);	\
+} while (0)
+
+#define down(l)			pthread_mutex_lock(&(l)->lock)
+#define down_trylock(l)		pthread_mutex_trylock(&(l)->lock)
+#define up(l)			pthread_mutex_unlock(&(l)->lock)
+
+#endif /* __LIBXFS_SEMA_H__ */
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 9f28fd90..1fc243cf 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -50,6 +50,7 @@
 #include "atomic.h"
 #include "spinlock.h"
 #include "completion.h"
+#include "sema.h"
 
 #include "xfs_types.h"
 #include "xfs_arch.h"
-- 
2.30.2

Re: [PATCH V2 1/5] xfsprogs: introduce liburcu support

[Eric Sandeen]

On 9/24/21 9:09 AM, Chandan Babu R wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The upcoming buffer cache rework/kerenl sync-up requires atomic
> variables. I could use C++11 atomics build into GCC, but they are a
> pain to work with and shoe-horn into the kernel atomic variable API.
> 
> Much easier is to introduce a dependency on liburcu - the userspace
> RCU library. This provides atomic variables that very closely match
> the kernel atomic variable API, and it provides a very similar
> memory model and memory barrier support to the kernel. And we get
> RCU support that has an identical interface to the kernel and works
> the same way.
> 
> Hence kernel code written with RCU algorithms and atomic variables
> will just slot straight into the userspace xfsprogs code without us
> having to think about whether the lockless algorithms will work in
> userspace or not. This reduces glue and hoop jumping, and gets us
> a step closer to having the entire userspace libxfs code MT safe.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> [chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]

Thanks for fixing that up. I had tried to use rcu_init like Dave originally
had, and I like that better in general, but I had trouble with it - I guess
maybe it gets redefined based on memory model magic and the actual symbol
"rcu_init" maybe isn't available? I didn't dig very much.

Also, dumb question from me - how do we know where we need the
rcu_[un]register_thread() calls? Will it be obvious if we miss it
in the future?  "each thread must invoke this function before its
first call to rcu_read_lock() or call_rcu()."

Thanks,
-Eric

Re: [PATCH V2 2/5] libxfs: add spinlock_t wrapper

[Eric Sandeen]

On 9/24/21 9:09 AM, Chandan Babu R wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> These provide the kernel spinlock_t interface, but are *not*
> spinlocks. Spinlocks cannot be used by general purpose userspace
> processes due to the fact they cannot control task preemption and
> scheduling reliability. Hence these are implemented as a
> pthread_mutex_t, similar to the way the kernel RT build implements
> spinlock_t as a kernel mutex.
> 
> Because the current libxfs spinlock "implementation" just makes
> spinlocks go away, we have to also add initialisation to spinlocks
> that libxfs uses that are missing from the userspace implementation.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> [chandan.babu@oracle.com: Initialize inode log item spin lock]
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>

...

> +/*
> + * This implements kernel compatible spinlock exclusion semantics. These,
> + * however, are not spinlocks, as spinlocks cannot be reliably implemented in
> + * userspace without using realtime scheduling task contexts. Hence this
> + * interface is implemented with pthread mutexes and so can block, but this is
> + * no different to the kernel RT build which replaces spinlocks with mutexes.
> + * Hence we know it works.
> + */
> +
> +typedef pthread_mutex_t	spinlock_t;
> +
> +#define spin_lock_init(l)	pthread_mutex_init(l, NULL)
> +#define spin_lock(l)           pthread_mutex_lock(l)
> +#define spin_trylock(l)        (pthread_mutex_trylock(l) != EBUSY)
> +#define spin_unlock(l)         pthread_mutex_unlock(l)

some whitespace mess here but I'll just clean that up.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Re: [PATCH V2 3/5] atomic: convert to uatomic

[Eric Sandeen]

On 9/24/21 9:09 AM, Chandan Babu R wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Now we have liburcu, we can make use of it's atomic variable
> implementation. It is almost identical to the kernel API - it's just
> got a "uatomic" prefix. liburcu also provides all the same aomtic
> variable memory barriers as the kernel, so if we pull memory barrier
> dependent kernel code across, it will just work with the right
> barrier wrappers.
> 
> This is preparation the addition of more extensive atomic operations
> the that kernel buffer cache requires to function correctly.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> [chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>   include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 54 insertions(+), 11 deletions(-)
> 
> diff --git a/include/atomic.h b/include/atomic.h
> index e0e1ba84..99cb85d3 100644
> --- a/include/atomic.h
> +++ b/include/atomic.h
> @@ -7,21 +7,64 @@
>   #define __ATOMIC_H__
>   
>   /*
> - * Warning: These are not really atomic at all. They are wrappers around the
> - * kernel atomic variable interface. If we do need these variables to be atomic
> - * (due to multithreading of the code that uses them) we need to add some
> - * pthreads magic here.
> + * Atomics are provided by liburcu.
> + *
> + * API and guidelines for which operations provide memory barriers is here:
> + *
> + * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
> + *
> + * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.

Given this, anyone have any objection to putting the #defines together at the
top, rather than hiding the 64 variants at the end of the file?

>    */
> +#include <urcu/uatomic.h>
> +#include "spinlock.h"
> +
>   typedef	int32_t	atomic_t;
>   typedef	int64_t	atomic64_t;
>   
> -#define atomic_inc_return(x)	(++(*(x)))
> -#define atomic_dec_return(x)	(--(*(x)))
> +#define atomic_read(a)		uatomic_read(a)
> +#define atomic_set(a, v)	uatomic_set(a, v)
> +#define atomic_add(v, a)	uatomic_add(a, v)
> +#define atomic_sub(v, a)	uatomic_sub(a, v)
> +#define atomic_inc(a)		uatomic_inc(a)
> +#define atomic_dec(a)		uatomic_dec(a)
> +#define atomic_inc_return(a)	uatomic_add_return(a, 1)
> +#define atomic_dec_return(a)	uatomic_sub_return(a, 1)
> +#define atomic_dec_and_test(a)	(atomic_dec_return(a) == 0)
> +#define cmpxchg(a, o, n)        uatomic_cmpxchg(a, o, n);

and I'll fix this whitespace.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Re: [PATCH V2 4/5] libxfs: add kernel-compatible completion API

[Eric Sandeen]

On 9/24/21 9:09 AM, Chandan Babu R wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> This is needed for the kernel buffer cache conversion to be able
> to wait on IO synchrnously. It is implemented with pthread mutexes
> and conditional variables.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

I am inclined to not merge patches 4 or 5 until there's something that
uses it. It can be merged and tested together with consumers, rather
than adding unused code at this point.  Thoughts?

-Eric

Re: [PATCH V2 1/5] xfsprogs: introduce liburcu support

[Chandan Babu R]

On 25 Sep 2021 at 03:21, Eric Sandeen wrote:
> On 9/24/21 9:09 AM, Chandan Babu R wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>> The upcoming buffer cache rework/kerenl sync-up requires atomic
>> variables. I could use C++11 atomics build into GCC, but they are a
>> pain to work with and shoe-horn into the kernel atomic variable API.
>> Much easier is to introduce a dependency on liburcu - the userspace
>> RCU library. This provides atomic variables that very closely match
>> the kernel atomic variable API, and it provides a very similar
>> memory model and memory barrier support to the kernel. And we get
>> RCU support that has an identical interface to the kernel and works
>> the same way.
>> Hence kernel code written with RCU algorithms and atomic variables
>> will just slot straight into the userspace xfsprogs code without us
>> having to think about whether the lockless algorithms will work in
>> userspace or not. This reduces glue and hoop jumping, and gets us
>> a step closer to having the entire userspace libxfs code MT safe.
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> [chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]
>
> Thanks for fixing that up. I had tried to use rcu_init like Dave originally
> had, and I like that better in general, but I had trouble with it - I guess
> maybe it gets redefined based on memory model magic and the actual symbol
> "rcu_init" maybe isn't available? I didn't dig very much.

Yes, rcu_init() gets defined to one of the variants of userspace rcu. Hence
there is no function named rcu_init().

>
> Also, dumb question from me - how do we know where we need the
> rcu_[un]register_thread() calls? Will it be obvious if we miss it
> in the future?  "each thread must invoke this function before its
> first call to rcu_read_lock() or call_rcu()."

Unfortunately, I don't think there is an obvious method to detect if calls to
rcu_[un]register_thread() is missing from any code changes implemented in the
future.

PS: I am no expert at RCU. My knowledge about RCU is limited to what I could
learn by reading articles on the world wide web.

-- 
chandan

Re: [PATCH V2 3/5] atomic: convert to uatomic

[Chandan Babu R]

On 25 Sep 2021 at 03:43, Eric Sandeen wrote:
> On 9/24/21 9:09 AM, Chandan Babu R wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>> Now we have liburcu, we can make use of it's atomic variable
>> implementation. It is almost identical to the kernel API - it's just
>> got a "uatomic" prefix. liburcu also provides all the same aomtic
>> variable memory barriers as the kernel, so if we pull memory barrier
>> dependent kernel code across, it will just work with the right
>> barrier wrappers.
>> This is preparation the addition of more extensive atomic operations
>> the that kernel buffer cache requires to function correctly.
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> [chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>>   include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 54 insertions(+), 11 deletions(-)
>> diff --git a/include/atomic.h b/include/atomic.h
>> index e0e1ba84..99cb85d3 100644
>> --- a/include/atomic.h
>> +++ b/include/atomic.h
>> @@ -7,21 +7,64 @@
>>   #define __ATOMIC_H__
>>     /*
>> - * Warning: These are not really atomic at all. They are wrappers around the
>> - * kernel atomic variable interface. If we do need these variables to be atomic
>> - * (due to multithreading of the code that uses them) we need to add some
>> - * pthreads magic here.
>> + * Atomics are provided by liburcu.
>> + *
>> + * API and guidelines for which operations provide memory barriers is here:
>> + *
>> + * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
>> + *
>> + * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.
>
> Given this, anyone have any objection to putting the #defines together at the
> top, rather than hiding the 64 variants at the end of the file?
>

I don't see any issue in doing that.

>>    */
>> +#include <urcu/uatomic.h>
>> +#include "spinlock.h"
>> +
>>   typedef	int32_t	atomic_t;
>>   typedef	int64_t	atomic64_t;
>>   -#define atomic_inc_return(x)	(++(*(x)))
>> -#define atomic_dec_return(x)	(--(*(x)))
>> +#define atomic_read(a)		uatomic_read(a)
>> +#define atomic_set(a, v)	uatomic_set(a, v)
>> +#define atomic_add(v, a)	uatomic_add(a, v)
>> +#define atomic_sub(v, a)	uatomic_sub(a, v)
>> +#define atomic_inc(a)		uatomic_inc(a)
>> +#define atomic_dec(a)		uatomic_dec(a)
>> +#define atomic_inc_return(a)	uatomic_add_return(a, 1)
>> +#define atomic_dec_return(a)	uatomic_sub_return(a, 1)
>> +#define atomic_dec_and_test(a)	(atomic_dec_return(a) == 0)
>> +#define cmpxchg(a, o, n)        uatomic_cmpxchg(a, o, n);
>
> and I'll fix this whitespace.
>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks for the reviews.

-- 
chandan

Re: [PATCH V2 4/5] libxfs: add kernel-compatible completion API

[Chandan Babu R]

On 25 Sep 2021 at 04:32, Eric Sandeen wrote:
> On 9/24/21 9:09 AM, Chandan Babu R wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>> This is needed for the kernel buffer cache conversion to be able
>> to wait on IO synchrnously. It is implemented with pthread mutexes
>> and conditional variables.
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>
> I am inclined to not merge patches 4 or 5 until there's something that
> uses it. It can be merged and tested together with consumers, rather
> than adding unused code at this point.  Thoughts?
>

I think I will let Dave answer this question since I believe he most likely
has a roadmap on when the consumers will land.

-- 
chandan

Re: [PATCH V2 1/5] xfsprogs: introduce liburcu support

[Dave Chinner]

On Fri, Sep 24, 2021 at 04:51:32PM -0500, Eric Sandeen wrote:
> On 9/24/21 9:09 AM, Chandan Babu R wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > The upcoming buffer cache rework/kerenl sync-up requires atomic
> > variables. I could use C++11 atomics build into GCC, but they are a
> > pain to work with and shoe-horn into the kernel atomic variable API.
> > 
> > Much easier is to introduce a dependency on liburcu - the userspace
> > RCU library. This provides atomic variables that very closely match
> > the kernel atomic variable API, and it provides a very similar
> > memory model and memory barrier support to the kernel. And we get
> > RCU support that has an identical interface to the kernel and works
> > the same way.
> > 
> > Hence kernel code written with RCU algorithms and atomic variables
> > will just slot straight into the userspace xfsprogs code without us
> > having to think about whether the lockless algorithms will work in
> > userspace or not. This reduces glue and hoop jumping, and gets us
> > a step closer to having the entire userspace libxfs code MT safe.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > [chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]
> 
> Thanks for fixing that up. I had tried to use rcu_init like Dave originally
> had, and I like that better in general, but I had trouble with it - I guess
> maybe it gets redefined based on memory model magic and the actual symbol
> "rcu_init" maybe isn't available? I didn't dig very much.

Ah, so I just checked where the m4/package_urcu.m4 file went -
forgot to re-add it after it rejected on apply. The diff is this:

diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
new file mode 100644
index 000000000000..9b0dee35d9a1
--- /dev/null
+++ b/m4/package_urcu.m4
@@ -0,0 +1,22 @@
+AC_DEFUN([AC_PACKAGE_NEED_URCU_H],
+  [ AC_CHECK_HEADERS(urcu.h)
+    if test $ac_cv_header_urcu_h = no; then
+       AC_CHECK_HEADERS(urcu.h,, [
+       echo
+       echo 'FATAL ERROR: could not find a valid urcu header.'
+       exit 1])
+    fi
+  ])
+
+AC_DEFUN([AC_PACKAGE_NEED_RCU_INIT],
+  [ AC_MSG_CHECKING([for liburcu])
+    AC_TRY_COMPILE([
+#define _GNU_SOURCE
+#include <urcu.h>
+    ], [
+       rcu_init();
+    ], liburcu=-lurcu
+       AC_MSG_RESULT(yes),
+       AC_MSG_RESULT(no))
+    AC_SUBST(liburcu)
+  ])

And this ends up with the output on a current debian system of:

checking urcu.h usability... yes
checking urcu.h presence... yes
checking for urcu.h... yes
checking for liburcu... yes

IOWs, the package check I was using directly probed for rcu_init()
being present in liburcu. Hence if liburcu is not providing this
function via the default linking like we use with xfsprogs, then
we fail at the configure step.

So it liburcu is not providing rcu_init(), then the configure step
should fail, and you need to work out what is different about the
liburcu that is installed on the distro you are building on....

> Also, dumb question from me - how do we know where we need the
> rcu_[un]register_thread() calls? Will it be obvious if we miss it
> in the future?  "each thread must invoke this function before its
> first call to rcu_read_lock() or call_rcu()."

urcu should fire an assert on any attempt to access urcu TLS storage
because the "urcu.registered" flag in the TLS area has not been
initialised.  IOWs, if we miss register/unregister then things will
go bad in urcu calls and it should be noticed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2 3/5] atomic: convert to uatomic

[Dave Chinner]

On Fri, Sep 24, 2021 at 05:13:30PM -0500, Eric Sandeen wrote:
> On 9/24/21 9:09 AM, Chandan Babu R wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Now we have liburcu, we can make use of it's atomic variable
> > implementation. It is almost identical to the kernel API - it's just
> > got a "uatomic" prefix. liburcu also provides all the same aomtic
> > variable memory barriers as the kernel, so if we pull memory barrier
> > dependent kernel code across, it will just work with the right
> > barrier wrappers.
> > 
> > This is preparation the addition of more extensive atomic operations
> > the that kernel buffer cache requires to function correctly.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > [chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
> > Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> > ---
> >   include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 54 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/atomic.h b/include/atomic.h
> > index e0e1ba84..99cb85d3 100644
> > --- a/include/atomic.h
> > +++ b/include/atomic.h
> > @@ -7,21 +7,64 @@
> >   #define __ATOMIC_H__
> >   /*
> > - * Warning: These are not really atomic at all. They are wrappers around the
> > - * kernel atomic variable interface. If we do need these variables to be atomic
> > - * (due to multithreading of the code that uses them) we need to add some
> > - * pthreads magic here.
> > + * Atomics are provided by liburcu.
> > + *
> > + * API and guidelines for which operations provide memory barriers is here:
> > + *
> > + * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
> > + *
> > + * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.
> 
> Given this, anyone have any objection to putting the #defines together at the
> top, rather than hiding the 64 variants at the end of the file?

I wanted to keep the -APIs- separate, because all the kernel
atomic/atomic64 stuff is already separate and type checked. I don't
see any point in commingling the two different atomic type APIs
just because the implementation ends up being the same and that some
wrappers are defines and others are static inline code.

Ideally, the wrappers should all be static inlines so we get correct
atomic_t/atomic64_t type checking in userspace. Those are the types
we care about in terms of libxfs, so to typecheck the API properly
these should -all- be static inlines. The patch as it stands was a
"get it working properly" patch, not a "finalised, strictly correct
API" patch. That was somethign for "down the road" as I polished the
patchset ready for eventual review.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2 3/5] atomic: convert to uatomic

[Eric Sandeen]

On 9/25/21 6:15 PM, Dave Chinner wrote:
> On Fri, Sep 24, 2021 at 05:13:30PM -0500, Eric Sandeen wrote:
>> On 9/24/21 9:09 AM, Chandan Babu R wrote:
>>> From: Dave Chinner <dchinner@redhat.com>
>>>
>>> Now we have liburcu, we can make use of it's atomic variable
>>> implementation. It is almost identical to the kernel API - it's just
>>> got a "uatomic" prefix. liburcu also provides all the same aomtic
>>> variable memory barriers as the kernel, so if we pull memory barrier
>>> dependent kernel code across, it will just work with the right
>>> barrier wrappers.
>>>
>>> This is preparation the addition of more extensive atomic operations
>>> the that kernel buffer cache requires to function correctly.
>>>
>>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>>> [chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
>>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>>> ---
>>>    include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
>>>    1 file changed, 54 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/atomic.h b/include/atomic.h
>>> index e0e1ba84..99cb85d3 100644
>>> --- a/include/atomic.h
>>> +++ b/include/atomic.h
>>> @@ -7,21 +7,64 @@
>>>    #define __ATOMIC_H__
>>>    /*
>>> - * Warning: These are not really atomic at all. They are wrappers around the
>>> - * kernel atomic variable interface. If we do need these variables to be atomic
>>> - * (due to multithreading of the code that uses them) we need to add some
>>> - * pthreads magic here.
>>> + * Atomics are provided by liburcu.
>>> + *
>>> + * API and guidelines for which operations provide memory barriers is here:
>>> + *
>>> + * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
>>> + *
>>> + * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.
>>
>> Given this, anyone have any objection to putting the #defines together at the
>> top, rather than hiding the 64 variants at the end of the file?
> 
> I wanted to keep the -APIs- separate, because all the kernel
> atomic/atomic64 stuff is already separate and type checked. I don't
> see any point in commingling the two different atomic type APIs
> just because the implementation ends up being the same and that some
> wrappers are defines and others are static inline code.
> 
> Ideally, the wrappers should all be static inlines so we get correct
> atomic_t/atomic64_t type checking in userspace. Those are the types
> we care about in terms of libxfs, so to typecheck the API properly
> these should -all- be static inlines. The patch as it stands was a
> "get it working properly" patch, not a "finalised, strictly correct
> API" patch. That was somethign for "down the road" as I polished the
> patchset ready for eventual review.....

Ok. Well, I was only talking about moving lines in your patch, nothing functional
at all. And ... that's why I had asked earlier (I think?) if your patch was
considered ready for review/merge, or just a demonstration of things to come.

So I guess changing it to a static inline as you suggest should be done before
merge.  Anything else like that that you don't actually consider quite ready,
in the first 3 patches?

Thanks,
-Eric

Re: [PATCH V2 3/5] atomic: convert to uatomic

[Dave Chinner]

On Sat, Sep 25, 2021 at 06:18:58PM -0500, Eric Sandeen wrote:
> On 9/25/21 6:15 PM, Dave Chinner wrote:
> > On Fri, Sep 24, 2021 at 05:13:30PM -0500, Eric Sandeen wrote:
> > > On 9/24/21 9:09 AM, Chandan Babu R wrote:
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > > 
> > > > Now we have liburcu, we can make use of it's atomic variable
> > > > implementation. It is almost identical to the kernel API - it's just
> > > > got a "uatomic" prefix. liburcu also provides all the same aomtic
> > > > variable memory barriers as the kernel, so if we pull memory barrier
> > > > dependent kernel code across, it will just work with the right
> > > > barrier wrappers.
> > > > 
> > > > This is preparation the addition of more extensive atomic operations
> > > > the that kernel buffer cache requires to function correctly.
> > > > 
> > > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > > [chandan.babu@oracle.com: Swap order of arguments provided to atomic[64]_[add|sub]()]
> > > > Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> > > > ---
> > > >    include/atomic.h | 65 ++++++++++++++++++++++++++++++++++++++++--------
> > > >    1 file changed, 54 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/include/atomic.h b/include/atomic.h
> > > > index e0e1ba84..99cb85d3 100644
> > > > --- a/include/atomic.h
> > > > +++ b/include/atomic.h
> > > > @@ -7,21 +7,64 @@
> > > >    #define __ATOMIC_H__
> > > >    /*
> > > > - * Warning: These are not really atomic at all. They are wrappers around the
> > > > - * kernel atomic variable interface. If we do need these variables to be atomic
> > > > - * (due to multithreading of the code that uses them) we need to add some
> > > > - * pthreads magic here.
> > > > + * Atomics are provided by liburcu.
> > > > + *
> > > > + * API and guidelines for which operations provide memory barriers is here:
> > > > + *
> > > > + * https://github.com/urcu/userspace-rcu/blob/master/doc/uatomic-api.md
> > > > + *
> > > > + * Unlike the kernel, the same interface supports 32 and 64 bit atomic integers.
> > > 
> > > Given this, anyone have any objection to putting the #defines together at the
> > > top, rather than hiding the 64 variants at the end of the file?
> > 
> > I wanted to keep the -APIs- separate, because all the kernel
> > atomic/atomic64 stuff is already separate and type checked. I don't
> > see any point in commingling the two different atomic type APIs
> > just because the implementation ends up being the same and that some
> > wrappers are defines and others are static inline code.
> > 
> > Ideally, the wrappers should all be static inlines so we get correct
> > atomic_t/atomic64_t type checking in userspace. Those are the types
> > we care about in terms of libxfs, so to typecheck the API properly
> > these should -all- be static inlines. The patch as it stands was a
> > "get it working properly" patch, not a "finalised, strictly correct
> > API" patch. That was somethign for "down the road" as I polished the
> > patchset ready for eventual review.....
> 
> Ok. Well, I was only talking about moving lines in your patch, nothing functional
> at all. And ... that's why I had asked earlier (I think?) if your patch was
> considered ready for review/merge, or just a demonstration of things to come.

I though you were asking "does it work/been tested enough to merge"
to which I answered "yes". I did point out that it was a quick
forward port, so stuff like variables the wrong way around in
wrappers I had to add for the forward port shouldn't be a surprise.

> So I guess changing it to a static inline as you suggest should be done before
> merge.

I don't see that as necessary before merging it. It's the direction
these wrappers need to move in so that we get better consistency in
libxfs between user and kernel space. We don't have to do everyone
at once and make code pristine perfect before merging it. Merging
functional, working code is far better than trying to polish off
every rough edge of every patch before considering them for merge..

> Anything else like that that you don't actually consider quite ready,
> in the first 3 patches?

Nope.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2 1/5] xfsprogs: introduce liburcu support

[Eric Sandeen]

On 9/25/21 6:05 PM, Dave Chinner wrote:
> On Fri, Sep 24, 2021 at 04:51:32PM -0500, Eric Sandeen wrote:
>> On 9/24/21 9:09 AM, Chandan Babu R wrote:
>>> From: Dave Chinner <dchinner@redhat.com>
>>>
>>> The upcoming buffer cache rework/kerenl sync-up requires atomic
>>> variables. I could use C++11 atomics build into GCC, but they are a
>>> pain to work with and shoe-horn into the kernel atomic variable API.
>>>
>>> Much easier is to introduce a dependency on liburcu - the userspace
>>> RCU library. This provides atomic variables that very closely match
>>> the kernel atomic variable API, and it provides a very similar
>>> memory model and memory barrier support to the kernel. And we get
>>> RCU support that has an identical interface to the kernel and works
>>> the same way.
>>>
>>> Hence kernel code written with RCU algorithms and atomic variables
>>> will just slot straight into the userspace xfsprogs code without us
>>> having to think about whether the lockless algorithms will work in
>>> userspace or not. This reduces glue and hoop jumping, and gets us
>>> a step closer to having the entire userspace libxfs code MT safe.
>>>
>>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>>> [chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]
>>
>> Thanks for fixing that up. I had tried to use rcu_init like Dave originally
>> had, and I like that better in general, but I had trouble with it - I guess
>> maybe it gets redefined based on memory model magic and the actual symbol
>> "rcu_init" maybe isn't available? I didn't dig very much.
> 
> Ah, so I just checked where the m4/package_urcu.m4 file went -
> forgot to re-add it after it rejected on apply. The diff is this:
> 
> diff --git a/m4/package_urcu.m4 b/m4/package_urcu.m4
> new file mode 100644
> index 000000000000..9b0dee35d9a1
> --- /dev/null
> +++ b/m4/package_urcu.m4
> @@ -0,0 +1,22 @@
> +AC_DEFUN([AC_PACKAGE_NEED_URCU_H],
> +  [ AC_CHECK_HEADERS(urcu.h)
> +    if test $ac_cv_header_urcu_h = no; then
> +       AC_CHECK_HEADERS(urcu.h,, [
> +       echo
> +       echo 'FATAL ERROR: could not find a valid urcu header.'
> +       exit 1])
> +    fi
> +  ])
> +
> +AC_DEFUN([AC_PACKAGE_NEED_RCU_INIT],
> +  [ AC_MSG_CHECKING([for liburcu])
> +    AC_TRY_COMPILE([
> +#define _GNU_SOURCE
> +#include <urcu.h>
> +    ], [
> +       rcu_init();
> +    ], liburcu=-lurcu
> +       AC_MSG_RESULT(yes),
> +       AC_MSG_RESULT(no))
> +    AC_SUBST(liburcu)
> +  ])

Works great here too. My error was not including urcu.h in the test I think,
and looking for the symbol directly. I will merge this version rather
than Chandan's.

Thanks,
-Eric

Re: [PATCH V2 4/5] libxfs: add kernel-compatible completion API

[Darrick J. Wong]

On Sat, Sep 25, 2021 at 03:59:00PM +0530, Chandan Babu R wrote:
> On 25 Sep 2021 at 04:32, Eric Sandeen wrote:
> > On 9/24/21 9:09 AM, Chandan Babu R wrote:
> >> From: Dave Chinner <dchinner@redhat.com>
> >> This is needed for the kernel buffer cache conversion to be able
> >> to wait on IO synchrnously. It is implemented with pthread mutexes
> >> and conditional variables.
> >> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> >
> > I am inclined to not merge patches 4 or 5 until there's something that
> > uses it. It can be merged and tested together with consumers, rather
> > than adding unused code at this point.  Thoughts?
> >
> 
> I think I will let Dave answer this question since I believe he most likely
> has a roadmap on when the consumers will land.

Technically speaking, one /could/ port xfs_scrub to use the kernel
completion API instead of calling pthread APIs directly, but I don't see
much gain from churning that.

--D

> -- 
> chandan

Re: [PATCH V2 4/5] libxfs: add kernel-compatible completion API

[Dave Chinner]

On Sat, Sep 25, 2021 at 03:59:00PM +0530, Chandan Babu R wrote:
> On 25 Sep 2021 at 04:32, Eric Sandeen wrote:
> > On 9/24/21 9:09 AM, Chandan Babu R wrote:
> >> From: Dave Chinner <dchinner@redhat.com>
> >> This is needed for the kernel buffer cache conversion to be able
> >> to wait on IO synchrnously. It is implemented with pthread mutexes
> >> and conditional variables.
> >> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> >
> > I am inclined to not merge patches 4 or 5 until there's something that
> > uses it. It can be merged and tested together with consumers, rather
> > than adding unused code at this point.  Thoughts?
> >
> 
> I think I will let Dave answer this question since I believe he most likely
> has a roadmap on when the consumers will land.

Don't care, as long as everyone agrees with the direction. FWIW, I'm
much more likely to use this completion interface in new userspace
code than I am to use pthreads directly...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2 1/5] xfsprogs: introduce liburcu support

[Eric Sandeen]

On 9/24/21 9:09 AM, Chandan Babu R wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The upcoming buffer cache rework/kerenl sync-up requires atomic
> variables. I could use C++11 atomics build into GCC, but they are a
> pain to work with and shoe-horn into the kernel atomic variable API.
> 
> Much easier is to introduce a dependency on liburcu - the userspace
> RCU library. This provides atomic variables that very closely match
> the kernel atomic variable API, and it provides a very similar
> memory model and memory barrier support to the kernel. And we get
> RCU support that has an identical interface to the kernel and works
> the same way.
> 
> Hence kernel code written with RCU algorithms and atomic variables
> will just slot straight into the userspace xfsprogs code without us
> having to think about whether the lockless algorithms will work in
> userspace or not. This reduces glue and hoop jumping, and gets us
> a step closer to having the entire userspace libxfs code MT safe.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> [chandan.babu@oracle.com: Add m4 macros to detect availability of liburcu]
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>

I've added Dave's m4 macros in place of yours, and with that circuitous
route I think we have a good set of changes, so:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

for the combined set of this-plus-dave's-m4's (elsewhere in replies
on this thread)

Thanks,
-Eric