Message-ID: <bc234db3-f4c9-ff5a-6c19-699c50514fc5@linux.ibm.com>
Date:   Fri, 17 Feb 2023 09:41:12 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v6 6/6] iommu/dma: Make flush queue sizes and timeout
 driver configurable
Content-Language: en-US
To:     Niklas Schnelle <schnelle@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>
References: <20230215120327.947336-1-schnelle@linux.ibm.com>
 <20230215120327.947336-7-schnelle@linux.ibm.com>
From:   Matthew Rosato <mjrosato@linux.ibm.com>
In-Reply-To: <20230215120327.947336-7-schnelle@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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 fewer
> flushes.
> 
> 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.
> 
> 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.
> 
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> #s390

> +#define S390_IOMMU_SINGLE_FQ_SIZE      32768
> +#define S390_IOMMU_SINGLE_FQ_TIMEOUT   1000
> +

One question about these values however, was there a rationale to choosing these particular numbers (anything worth documenting?) or were they were simply chosen because they showed similar characteristics to the previous DMA approach?  I'm mostly wondering if it's worth experimenting with other values here in the future to see what kind of impact it would have.