Page MenuHomeFreeBSD

ntsync(9)
Needs ReviewPublic

Authored by kib on Sun, May 17, 7:08 AM.
Tags
None
Referenced Files
F157291321: D57038.id178165.diff
Wed, May 20, 1:55 AM
F157258388: D57038.diff
Tue, May 19, 7:11 PM
F157228561: D57038.diff
Tue, May 19, 12:31 PM
Unknown Object (File)
Mon, May 18, 4:40 PM
Unknown Object (File)
Mon, May 18, 10:29 AM
Unknown Object (File)
Mon, May 18, 8:42 AM
Unknown Object (File)
Mon, May 18, 1:47 AM
Unknown Object (File)
Sun, May 17, 11:13 PM
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This code passes the Linux ntsync tests, ported to FreeBSD at https://github.com/kostikbel/freebsd-ntsync-test

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Sun, May 17, 7:08 AM
kib added a subscriber: emaste.

Report kinfo for ntsync filedescs.
Minor tweaks overall.

Report some info on stat(2)

The last time this problem for WINE was discussed, before Linux shipped NTSYNC, there was some consensus that various _umtx_op operations should be exposed to kqueue. Unfortunately, this coincided with a refactoring of the umtx code to unify it with the Linuxulator’s futex support and so the patches bitrotted.

This change seems to introduce an entirely new subsystem, independent of umtx. It would be nice if the documentation included rationale for why the NTSYNC types are not new umtx types and need a new device and set of ioctls (outside Linuxulator support, where they’re needed there for binary compatibility).

I'm not sure the problem (of ntsync compatibility) even existed before Linux shipped ntsync, so it's not clear how it could be discussed. Any references to that? futex2 is entirely independent thing that is no longer going to be used by Proton's Wine variant and Wine definitely doesn't have any interest in kqueue.

I'm not sure the problem (of ntsync compatibility) even existed before Linux shipped ntsync

That is not the underlying problem. There were two closely related problems:

  • In general, it is useful to be able to sleep waiting for in-process and external events on the same run loop. EVFILT_USER was a poor approximation of what you actually wanted.
  • WINE, in particular, wanted to emulate Windows system calls that allow waiting on multiple mutexes or semaphores at a time. NT has supported WaitForMultipleObjects since XP (maybe earlier?) and WINE needed to implement these.

The previous discussion was motivated by the latter, but with the goal of supporting the latter.

Linux's NTSYNC is specifically tailored to the latter.

I don't doubt kqueue's usefulness, it's just that I don't remember and I'm unable to find any discussions.

Add linux compat code. Cannot test with linux test, since it uses pidfd calls which are not implemented.

linux compat mostly works.
Match native FreeBSD ioctl args with Linux counterparts.

Make tests in linuxolator pass fully.

From build testing with the latest changes, I've ran into 3 build errors and have suggested fixes for each:

Error 1:

/usr/src/sys/dev/ntsync/ntsync.c:255:22: error: variable 'priv' set but not used [-Werror,-Wunused-but-set-variable]
  255 |         struct ntsync_priv *priv;
      |                             ^
1 error generated.
*** [ntsync.o] Error code 1

Fix:

diff --git a/sys/dev/ntsync/ntsync.c b/sys/dev/ntsync/ntsync.c
index 2cd9d33f85d1..829f8b2c928a 100644
--- a/sys/dev/ntsync/ntsync.c
+++ b/sys/dev/ntsync/ntsync.c
@@ -255,7 +255,7 @@ ntsync_wait(struct ntsync_wait_state *state, struct thread *td)
 static void
 ntsync_wakeup_waiters(struct ntsync_obj *obj)
 {
-	struct ntsync_priv *priv;
+	struct ntsync_priv *priv __diagused;
 	struct ntsync_obj_waiter *w;
 
 	priv = obj->owner;

Error 2:

--- includes_subdir_include ---
install: target directory `/usr/obj/usr/src/amd64.amd64/tmp/usr/include/dev/ntsync' does not exist
usage: install [-bCcpSsUv] [-f flags] [-g group] [-m mode] [-o owner]
               [-M log] [-D dest] [-h hash] [-T tags]
               [-B suffix] [-l linkflags] [-N dbdir]
               file1 file2
       install [-bCcpSsUv] [-f flags] [-g group] [-m mode] [-o owner]
               [-M log] [-D dest] [-h hash] [-T tags]
               [-B suffix] [-l linkflags] [-N dbdir]
               file1 ... fileN directory
       install -dU [-vU] [-g group] [-m mode] [-N dbdir] [-o owner]
               [-M log] [-D dest] [-h hash] [-T tags]
               directory ...
*** [symlinks] Error code 64

make[3]: stopped making "includes" in /usr/src/include

Fix:

diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.dist
index 97f2194a3fa1..9c64afb87c9d 100644
--- a/etc/mtree/BSD.include.dist
+++ b/etc/mtree/BSD.include.dist
@@ -157,6 +157,8 @@
             mpilib
             ..
         ..
+        ntsync
+        ..
         nvme
         ..
         nvmf

Error 3:

In file included from /usr/src/sys/dev/ntsync/linux_ntsync.c:22:
/usr/src/sys/compat/linux/linux_common.h:31:42: error: declaration of 'struct ifnet' will not be visible outside of this function [-Werror,-Wvisibility]
   31 | int     ifname_bsd_to_linux_ifp(const struct ifnet *, char *, size_t);
      |                                              ^
/usr/src/sys/compat/linux/linux_common.h:40:1: error: unknown type name 'sa_family_t'; did you mean '__sa_family_t'?
   40 | sa_family_t     linux_to_bsd_domain(sa_family_t domain);
      | ^~~~~~~~~~~
      | __sa_family_t
/usr/src/sys/sys/_types.h:152:19: note: '__sa_family_t' declared here
  152 | typedef __uint8_t       __sa_family_t;
      |                         ^
In file included from /usr/src/sys/dev/ntsync/linux_ntsync.c:22:
/usr/src/sys/compat/linux/linux_common.h:40:33: error: unknown type name 'sa_family_t'; did you mean '__sa_family_t'?
   40 | sa_family_t     linux_to_bsd_domain(sa_family_t domain);
      |                                     ^~~~~~~~~~~
      |                                     __sa_family_t
/usr/src/sys/sys/_types.h:152:19: note: '__sa_family_t' declared here
  152 | typedef __uint8_t       __sa_family_t;
      |                         ^
In file included from /usr/src/sys/dev/ntsync/linux_ntsync.c:22:
/usr/src/sys/compat/linux/linux_common.h:41:1: error: unknown type name 'sa_family_t'; did you mean '__sa_family_t'?
   41 | sa_family_t     bsd_to_linux_domain(sa_family_t domain);
      | ^~~~~~~~~~~
      | __sa_family_t
/usr/src/sys/sys/_types.h:152:19: note: '__sa_family_t' declared here
  152 | typedef __uint8_t       __sa_family_t;
      |                         ^
In file included from /usr/src/sys/dev/ntsync/linux_ntsync.c:22:
/usr/src/sys/compat/linux/linux_common.h:41:33: error: unknown type name 'sa_family_t'; did you mean '__sa_family_t'?
   41 | sa_family_t     bsd_to_linux_domain(sa_family_t domain);
      |                                     ^~~~~~~~~~~
      |                                     __sa_family_t
/usr/src/sys/sys/_types.h:152:19: note: '__sa_family_t' declared here
  152 | typedef __uint8_t       __sa_family_t;
      |                         ^
In file included from /usr/src/sys/dev/ntsync/linux_ntsync.c:22:
/usr/src/sys/compat/linux/linux_common.h:43:41: error: declaration of 'struct sockaddr' will not be visible outside of this function [-Werror,-Wvisibility]
   43 | int             bsd_to_linux_sockaddr(const struct sockaddr *sa,
      |                                                    ^
/usr/src/sys/compat/linux/linux_common.h:44:32: error: unknown type name 'socklen_t'
   44 |                     struct l_sockaddr **lsa, socklen_t len);
      |                                              ^
/usr/src/sys/compat/linux/linux_common.h:46:14: error: declaration of 'struct sockaddr' will not be visible outside of this function [-Werror,-Wvisibility]
   46 |                     struct sockaddr **sap, socklen_t *len);
      |                            ^
/usr/src/sys/compat/linux/linux_common.h:46:30: error: unknown type name 'socklen_t'
   46 |                     struct sockaddr **sap, socklen_t *len);
      |                                            ^
9 errors generated.
*** [linux_ntsync.o] Error code 1

Fix:

diff --git a/sys/dev/ntsync/linux_ntsync.c b/sys/dev/ntsync/linux_ntsync.c
index 17487ea20ea4..ad8b12b9e486 100644
--- a/sys/dev/ntsync/linux_ntsync.c
+++ b/sys/dev/ntsync/linux_ntsync.c
@@ -14,7 +14,9 @@
 #include <sys/kernel.h>
 #include <sys/module.h>
 #include <sys/proc.h>
+#include <sys/socket.h>
 #include <sys/vnode.h>
+#include <net/if.h>
 #include <dev/ntsync/ntsyncvar.h>
 
 #include <machine/../linux/linux.h>

After these changes, I was able to successfully buildworld+buildkernel.

From build testing with the latest changes, I've ran into 3 build errors and have suggested fixes for each:

Thank you for the report. I fixed the '__diagused' case differently, the local is not needed.
For the ntsync_linux..c errors, it seems that you do not have the source commit f6f5eb3190165cea44700.

Fix !DEBUG build.
Fix headers installation.

In D57038#1308842, @kib wrote:

Thank you for the report. I fixed the '__diagused' case differently, the local is not needed.
For the ntsync_linux..c errors, it seems that you do not have the source commit f6f5eb3190165cea44700.

Thank you as well. Your updated changes and cherry-picking your commit did the trick for me. 😄