Message-ID: <3618028df347d352ca6b3be306c1a21d74899298.camel@linux.ibm.com>
Subject: Re: [PATCH v6 6/6] iommu/dma: Make flush queue sizes and timeout
 driver configurable
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     Matthew Rosato <mjrosato@linux.ibm.com>,
        Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Wenjia Zhang <wenjia@linux.ibm.com>
Cc:     Gerd Bayer <gbayer@linux.ibm.com>,
        Pierre Morel <pmorel@linux.ibm.com>, iommu@lists.linux.dev,
        linux-s390@vger.kernel.org, borntraeger@linux.ibm.com,
        hca@linux.ibm.com, gor@linux.ibm.com,
        gerald.schaefer@linux.ibm.com, agordeev@linux.ibm.com,
        svens@linux.ibm.com, linux-kernel@vger.kernel.org,
        Julian Ruess <julianr@linux.ibm.com>
Date:   Fri, 17 Feb 2023 16:40:59 +0100
In-Reply-To: <bc234db3-f4c9-ff5a-6c19-699c50514fc5@linux.ibm.com>
References: <20230215120327.947336-1-schnelle@linux.ibm.com>
         <20230215120327.947336-7-schnelle@linux.ibm.com>
         <bc234db3-f4c9-ff5a-6c19-699c50514fc5@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.3 (3.46.3-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-02-17 at 09:41 -0500, Matthew Rosato wrote:
> On 2/15/23 7:03 AM, Niklas Schnelle wrote:
> > Flush queues currently use a fixed compile time size of 256 entries.
> > This being a power of 2 allows the compiler to use shift and mask
> > instead of more expensive modulo operations. With per-CPU flush queues
> > larger queue sizes would hit per-CPU allocation limits, with a single
> > flush queue these limits do not apply however. Also with single queues
> > being particularly suitable for virtualized environments with expensive
> > IOTLB flushes these benefit especially from larger queues and thus fewe=
r
> > flushes.
> >=20
> > To this end re-order struct iova_fq so we can use a dynamic array and
> > introduce the flush queue size and timeouts as new options in the
> > dma_iommu_options struct. So as not to lose the shift and mask
> > optimization, check that the variable length is a power of 2 and use
> > explicit shift and mask instead of letting the compiler optimize this.
> >=20
> > In the s390 IOMMU driver a large fixed queue size and timeout is then
> > set together with single queue mode bringing its performance on s390
> > paged memory guests on par with the previous s390 specific DMA API
> > implementation.
> >=20
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
>=20
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> #s390
>=20
> > +#define S390_IOMMU_SINGLE_FQ_SIZE      32768
> > +#define S390_IOMMU_SINGLE_FQ_TIMEOUT   1000
> > +
>=20
> One question about these values however, was there a rationale to choosin=
g these particular numbers (anything worth documenting?) or were they were =
simply chosen because they showed similar characteristics to the previous D=
MA approach?  I'm mostly wondering if it's worth experimenting with other v=
alues here in the future to see what kind of impact it would have.
>=20

For the flush queue size, which has to be a power of two, I basically
picked the smallest value that gave me similar performance under z/VM
as prior to the conversion. Somewhat similar for the timeout though I
tried only a dozen "nice" values. Since the timeout basically
determines how long a rogue PCI device could potentially access old
data I found that 1 second is also kind of easy to explain. But yes
these could still be tuned in the future.

Thanks,
Niklas