On Tue, Jun 13, 2023 at 08:45:31AM +0200, Juergen Gross wrote: > On 12.06.23 22:09, Demi Marie Obenour wrote: > > On Mon, Jun 12, 2023 at 08:27:59AM +0200, Juergen Gross wrote: > > > On 10.06.23 17:32, Demi Marie Obenour wrote: > > > > When a grant entry is still in use by the remote domain, Linux must put > > > > it on a deferred list. > > > > > > This lacks quite some context. > > > > > > The main problem is related to the grant not having been unmapped after > > > the end of a request, but the side granting the access is assuming this > > > should be the case. > > > > The GUI agent has relied on deferred grant reclaim for as long as it has > > existed. One could argue that doing so means that the agent is misusing > > gntalloc, but this is not documented anywhere. A better fix would be to > > use IOCTL_GNTDEV_SET_UNMAP_NOTIFY in the GUI daemon. > > I don't think this is a gntalloc specific problem. You could create the same > problem with a kernel implementation. > > And yes, using IOCTL_GNTDEV_SET_UNMAP_NOTIFY might be a good idea. Just a comment to this one: while in practice it might work most of the time, in theory there could be hundreds or even thousands of mappings at the same time and event channels seems to be too scarce resource to get unmap notification for all of those (even if you'd use an event channel per grant area, not individual page). As said to Demi elsewhere, our GUI protocol might have a way to signal when unmapping happened, but the backend (the side mapping the grants) would need to know when they actually were unmapped by another process (in some configurations, like Wayland mentioned below, the process itself doesn't give us this info). Ideally, for this we'd need something similar to IOCTL_GNTDEV_SET_UNMAP_NOTIFY that notifies the mapping side, not the other domain. I have no idea how such API should look (just poll()/read() on the gntdev FD used for that IOCTL?) or whether it's really worth adding such API. Or maybe something like this already exists? In worst case, even polling for this info (so - a way to check if process P has grant G from domain D still mapped) would do, although that wouldn't be ideal. > > > In general this means that the two sides implementing the protocol don't > > > agree how it should work, or that the protocol itself has a flaw. > > > > What would a better solution be? This is going to be particularly > > tricky with Wayland, as the wl_shm protocol makes absolutely no > > guarantees that compositors will promptly release the mapping and > > provides no way whatsoever for Wayland clients to know when this has > > happened. Relying on an LD_PRELOAD hack is not sustainable. Anyway, besides trying to fix our usage of grant tables, I think it's still worthwhile to improve deferred reclaim, either by making the limit per iteration configurable (sysfs node / writable module parameter is fine) or by making the limit not needed by using workqueue. The former sounds like a smaller code change. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
Attachment:
signature.asc
Description: PGP signature
