Use FD passing for shared memory #78
Conversation
DemiMarie
force-pushed
the
xcb-fd-passing
branch
from
0aa4935 to
b29d0be
Compare
|
This actually works now! I am still not sure if the extra complexity (and overhead) is worthwhile, but this shows that the approach is viable. Tray icons have not been tested. |
|
Last few commits add some consistency checks and fix a premature free() in shmoverride, which led to every element of shmoverride’s global list being located in freed memory. |
DemiMarie
force-pushed
the
xcb-fd-passing
branch
8 times, most recently
from
c93b2c4 to
d0bb7d3
Compare
|
PipelineRetry |
|
This has passed every test I can think of, which proves that shmoverride will not be needed at all for Wayland. |
|
PipelineRefresh |
|
Windows qubes work, too! |
|
I'm going to leave it open until after branching off R4.1. |
DemiMarie
force-pushed
the
xcb-fd-passing
branch
6 times, most recently
from
78a0c65 to
54ce194
Compare
| #if defined MAP_ANON && defined MAP_ANONYMOUS && (MAP_ANONYMOUS) != (MAP_ANON) | ||
| # error header bug (def mismatch) | ||
| #endif | ||
| #ifndef MAP_ANON | ||
| # define MAP_ANON 0 | ||
| #endif | ||
| #ifndef MAP_ANONYMOUS | ||
| # define MAP_ANONYMOUS 0 | ||
| #endif |
There was a problem hiding this comment.
How much of this section is necessary?
DemiMarie
force-pushed
the
xcb-fd-passing
branch
from
54ce194 to
fc50d2f
Compare
DemiMarie
force-pushed
the
xcb-fd-passing
branch
2 times, most recently
from
c66b5d8 to
57e7a3a
Compare
DemiMarie
force-pushed
the
xcb-fd-passing
branch
from
57e7a3a to
2041781
Compare
|
PipelineRetry |
|
@marmarek please re-review |
|
GUI daemon crashes with |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.1&build=2022100203-4.1&flavor=pull-requests New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.1&build=2022092706-4.1&flavor=update
Failed tests40 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/46840#dependencies 1 fixed
Unstable tests
|
That was due to an incorrect check for the return value of |
DemiMarie
force-pushed
the
xcb-fd-passing
branch
from
c108579 to
ef6fa7a
Compare
This avoids having to define XLIB_ILLEGAL_ACCESS, which we would prefer to avoid.
Xlib does not support file descriptor passing or dma-bufs. The GUI daemon does not currently use either of these, but it will in the future. Switching from Xlib to XCB is a necessary first step in that direction.
Referencing a function that the GUI daemon no longer uses would be confusing.
This is another attempt at using FD passing for shared memory, at the cost of still requiring shmoverride. The reason I am filing it as a PR is to at least give it a chance to be reviewed, so that the work done on it has a chance of not being wasted.
By sending a freshly opened Xen file descriptor at each request, the offset is guaranteed to be 0. Furthermore, there is no need to call IOCTL_GNTDEV_UNMAP_GRANT_REF after munmap() ― just calling munmap() is sufficient. The only remaining reason to override munmap() is to handle mfn-based mappings, which may have an offset that is not a multiple of the page size. These require correcting the address before the kernel will accept the munmap() call. Fortunately, there is no legitimate reason for the X.org Server to call munmap() with a non-page-aligned address, so it is safe to unconditionally align the address and pass the aligned version to the kernel, adjusting the size accordingly. Therefore, it is no longer necessary for shmoverride to maintain any state whatsoever about the current mappings, simplifying the code enormously. No mutex locking is needed, and no list of mapped pages needs to be kept. The result is that the code is simpler and less prone to bugs.
DemiMarie
force-pushed
the
xcb-fd-passing
branch
from
ef6fa7a to
efab29b
Compare
|
Closing in favor of #116. |
This is another attempt at using FD passing for shared memory, at the cost of still requiring shmoverride.
This doesn’t actually work, and it may not be a good use of time even if it did. The reason I am filing it as a PR is to at least give it a chance to be reviewed, so that the work done on it has a chance of not being wasted.The code is now fully functional and has been tested over VNC. The main advantage is that the same approach works on Wayland, which does not support System V IPC. Newer versions of glibc (which do not use the legacy__fxstatand__fxstat64functions) are known to build but have not been otherwise tested.