Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i1568416f:Fastmail
Date:   Mon, 19 Jun 2023 15:23:10 +0200
From:   Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
To:     Juergen Gross <jgross@suse.com>
Cc:     Demi Marie Obenour <demi@invisiblethingslab.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Jan Beulich <JBeulich@suse.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Xen developer discussion <xen-devel@lists.xenproject.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        stable@vger.kernel.org
Subject: Re: [PATCH] xen: speed up grant-table reclaim
Message-ID: <ZJBWvpt3UAxeIC13@mail-itl>
References: <20230610153232.1859-1-demi@invisiblethingslab.com>
 <85275900-6b6a-5391-a2a0-16704df3f00f@suse.com>
 <ZId7ixbqlCSygtvb@itl-email>
 <1150fd9c-25c8-c91c-4ca2-2c587e3fbb43@suse.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="oN+TsMtwsmB08J9P"
Content-Disposition: inline
In-Reply-To: <1150fd9c-25c8-c91c-4ca2-2c587e3fbb43@suse.com>
Precedence: bulk


--oN+TsMtwsmB08J9P
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Date: Mon, 19 Jun 2023 15:23:10 +0200
From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= <marmarek@invisiblethingslab.com>
To: Juergen Gross <jgross@suse.com>
Cc: Demi Marie Obenour <demi@invisiblethingslab.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
	Jan Beulich <JBeulich@suse.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Xen developer discussion <xen-devel@lists.xenproject.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH] xen: speed up grant-table reclaim

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.
> > >=20
> > > This lacks quite some context.
> > >=20
> > > The main problem is related to the grant not having been unmapped aft=
er
> > > the end of a request, but the side granting the access is assuming th=
is
> > > should be the case.
> >=20
> > 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.
>=20
> I don't think this is a gntalloc specific problem. You could create the s=
ame
> problem with a kernel implementation.
>=20
> 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 do=
n't
> > > agree how it should work, or that the protocol itself has a flaw.
> >=20
> > 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.

--=20
Best Regards,
Marek Marczykowski-G=C3=B3recki
Invisible Things Lab

--oN+TsMtwsmB08J9P
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmSQVr4ACgkQ24/THMrX
1yyh2gf+NouvKQSmSXw0WMD1FLa4ZACvsVdcsJy7Prup6EjdKYT/5hicLhNOM7Os
0ctvuvVYt1gEUrJu/BsWgCd2u55yYEe4VH2oeqiDJF3kqhUFOTRCzpWzr3+3COzf
e4gzlHfhwc82S9VrvhwGsCOiWRv+BduUa3x/AlWufrDjloIOYZ4tWuaCptYh/vnP
9khLdrg7gWDFXbslRW21rrPkuQDKqLe9E4q4bgBlgjKMQd9Qyw6KxyF+gd5I0WDl
BPWJnIqX7ps4bnxNQw7r6VvIjkd9UhMG9pryM4mYSC5/+oO86TOwuboBqWw+SvHJ
OXhGeB0gN1QZxMuvnp9ipUIci3NIcw==
=7szl
-----END PGP SIGNATURE-----

--oN+TsMtwsmB08J9P--