[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 03/15] IOMMU/x86: support freeing of pagetables
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 1 Jun 2022 11:24:08 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=QwFMEiCsktYe+ykj4mHdBcUXkDA/aI5nE5HdboBnRqU=; b=ln73o00c9PZRGcyZRCfempVcmKmZjWdizPpuz8s7S5cB8iXXCrZCKWHnXkKeIfSlJwXSAysj76kzE9BZvINpAO9PgdMNWQ170xdV4pplIiHTGBN9wVBeGBxBS3BOkhyBHF/fS1mNIDE5fdEvpuorfYZ4noURZeYJyWN96irNvIUf07pPlELOcVSi3TYWVzs7jz8+5rLcZWLyy/Xk5/Iu74eKw0nGOicHvCFzZH3GV3E+j4Ue+3kWWVImsvV5fzL0ikoUJNWdyA3KDG1lX9kJdcdsQ+S0rhmJH73pHm6pAEMDng/WU6RXmVeWo8teok9t24BLSc2QAxgo5WR7gPFtmg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eqTquFLpCVmPAEk791LxTZ8zofhFwPQwT+nxrpFgN84BlP9UVW/43fmKu6OHx3kcoM4Jb61sNyCyTBXrGt4MuArMZjyNfmoFuExu3OOSv6Lr1xlWn+x7kyEcisYsyHheLW60Ph+ZnmRsa3iMNKP9F+VvSr/LeIg++vvLs81++vPEV1TrkF3WH3bO7g97+97JhcO+/XdvYwT45moeEdNVi4g5BJ6p8H0XbHzT1QbnIQlpbxdDXso+p/A4e1fGEEJHHxDJMFKK3J4s+EsnG4lOJ1MeORne0OiuqqheoDwx1U0bKrwuh+KufugkHX2s3oMIX0/f467XSB6x6isDRly3jw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
- Delivery-date: Wed, 01 Jun 2022 09:24:35 +0000
- Ironport-data: A9a23:Z9YrYqOs0YY5SsHvrR3TlsFynXyQoLVcMsEvi/4bfWQNrUpw0T1Wz GRKWDjXbPnZMWqhKookPdji9xtXvMCHzYRhTQto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdleF+lH1dOKJQUBUjclkfJKlYAL/En03FFYMpBsJ00o5wbZn2tcw2LBVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Z0 Nxwu766S1sVNaTRltlBCjlGLiAuIvgTkFPHCSDXXc276WTjKyep5so0SUY8MMsf5/p9BnxI+ boAMjcRYxufhuWwhrWmVu1rgcdlJ87uVG8dkig4kXeFUrB5GtaaHPuiCdxwhV/cguhUGvnTf YwBYCdHZxXceRxffFwQDfrSmc/32yCkLGYH9zp5o4II3UHf6BRtk4SuOYb1a42GGdlMolSh8 zeuE2PRR0ty2Mak4TiP/2+oh+TPtTjmQ49UH7q9ntZ6jVvWymENBRk+UVqgveL/mkO4Q8hYK UEf5mwpt6da3FSiU93VTxC+5nmesXYht8F4FuQ77ESI1fDS6gPBVmwcFGceNpohqdM8QiEs2 hmRhdT1CDdzsbqTD3WA6rOTqjD0Mi8QRYMfWRI5ocI+y4GLiOkOYtjnF76PzIbdYgXJJAzN
- Ironport-hdrordr: A9a23:VT6P6a+4GiCyykYIVkpuk+FKdb1zdoMgy1knxilNoENuH/Bwxv rFoB1E73TJYVYqN03IV+rwXZVoZUmsjaKdhrNhRotKPTOWwVdASbsP0WKM+V3d8kHFh41gPO JbAtJD4b7LfCdHZKTBkW6F+r8bqbHokZxAx92uqUuFJTsaF52IhD0JbjpzfHcGJjWvUvECZe ehD4d81nOdUEVSSv7+KmgOXuDFqdGOvJX6YSQeDxpizAWVlzun5JPzDhDdh34lInhy6IZn1V KAvx3y562lvf3+4hjA11XL55ATvNf60NNMCOGFl8BQADTxjQSDYphnRtS5zXgIidDqzGxvvM jHoh8mMcg2w3TNflutqR+o4AXk2CZG0Q6X9XaoxV/Y5eDpTjMzDMRMwahDdAHC1kYmtNZglI pWwmOwrfNsfFz9tRW4w+KNewBhl0Kyr3Znu/UUlWZjXYwXb6IUhZAD/XlSDIwLEEvBmcwa+d FVfYDhDcttABOnhyizhBgt/DXsZAV/Iv6+eDlNhiTPuAIm3kyQzCMjtbkidzk7hdcAoqJ/lp X525RT5c9zp/AtHNJA7cc6MLyK4z/2MGTx2Fz7GyWVKIg3f1TwlrXQ3JIZoMmXRb1g9upBpH 2GaiITiVIP
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Jun 01, 2022 at 09:32:44AM +0200, Jan Beulich wrote:
> On 31.05.2022 18:25, Roger Pau Monné wrote:
> > On Fri, May 27, 2022 at 01:13:09PM +0200, Jan Beulich wrote:
> >> @@ -566,6 +567,98 @@ struct page_info *iommu_alloc_pgtable(st
> >> return pg;
> >> }
> >>
> >> +/*
> >> + * Intermediate page tables which get replaced by large pages may only be
> >> + * freed after a suitable IOTLB flush. Hence such pages get queued on a
> >> + * per-CPU list, with a per-CPU tasklet processing the list on the
> >> assumption
> >> + * that the necessary IOTLB flush will have occurred by the time tasklets
> >> get
> >> + * to run. (List and tasklet being per-CPU has the benefit of accesses not
> >> + * requiring any locking.)
> >> + */
> >> +static DEFINE_PER_CPU(struct page_list_head, free_pgt_list);
> >> +static DEFINE_PER_CPU(struct tasklet, free_pgt_tasklet);
> >> +
> >> +static void free_queued_pgtables(void *arg)
> >> +{
> >> + struct page_list_head *list = arg;
> >> + struct page_info *pg;
> >> + unsigned int done = 0;
> >> +
> >> + while ( (pg = page_list_remove_head(list)) )
> >> + {
> >> + free_domheap_page(pg);
> >> +
> >> + /* Granularity of checking somewhat arbitrary. */
> >> + if ( !(++done & 0x1ff) )
> >> + process_pending_softirqs();
> >
> > Hm, I'm wondering whether we really want to process pending softirqs
> > here.
> >
> > Such processing will prevent the watchdog from triggering, which we
> > likely want in production builds. OTOH in debug builds we should make
> > sure that free_queued_pgtables() doesn't take longer than a watchdog
> > window, or else it's likely to cause issues to guests scheduled on
> > this same pCPU (and calling process_pending_softirqs() will just mask
> > it).
>
> Doesn't this consideration apply to about every use of the function we
> already have in the code base?
Not really, at least when used by init code or by the debug key
handlers. This use is IMO different than what I would expect, as it's
a guest triggered path that we believe do require such processing.
Normally we would use continuations for such long going guest
triggered operations.
Thanks, Roger.
|