lkml.org 
[lkml]   [2022]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] iommu/iova: using separate rcache for SAC and DAC
From
On 2022-09-19 04:02, brookxu.cn@gmail.com wrote:
>
>
> 在 2022/9/17 1:03, Robin Murphy 写道:
>> On 2022-09-16 16:46, brookxu.cn wrote:
>>> From: Chunguang Xu <chunguang.xu@shopee.com>
>>>
>>> While iommu_dma_forcedac disable, for PCI device kernel
>>> try SAC first, if failed then try DAC. Since now rcache
>>> does not distinguish SAC and DAC, if all PFNs contained
>>> in cpu loaded cache is larger than SAC max PFN, but the
>>> SAC address space is sufficient, as cpu loaded cached is
>>> not empty, kernel will iova_alloc () to alloc IOVA. For
>>> PCI device, kernel alloc SAC most, loaded cache may
>>> invalid for SAC alloc for a long time, kernel will enter
>>> alloc_iova() slow path frequencely, as result performance
>>> is degrade. To circumvent this problem, SAC and DAC maybe
>>> better to use separate caches.
>>
>> I really dislike the idea of doubling the already significantly large
>> footprint of rcaches purely to optimise for the stupid case. If you've
>> got to the point of hitting contention in the SAC path, you presumably
>> don't have broken hardware/drivers so you're infinitely better off
>> using forcedac and avoiding it entirely to begin with. And frankly
>> even if you *do* have broken hardware, if you care about performance
>> you're still better off fixing the relevant driver(s) to set correct
>> DMA masks so you can use forcedac.
>
> Some of our NICs have poor performance after heavy traffic impact. We
> found that it maybe caused by a large number of 1-2 order IOVA SAC
> address applications to make SAC address space fragmentation during
> taffic impact, which in turn causes some 3 order IOVA SAC address
> applications to fail. Kernel will try DAC address and release it into
> rcache. After the traffic recovery, and SAC address space recovery from
> fragmentation, but due to the existence of several DAC PFN on the cache
> of a certain CPU,  which will broken the usage of rcache for SAC address
> application. As a result, kernel will enter alloc_iova() slow path
> frequencely and performance is poor. So I think this issue should be not
> too much of a broken hardware or driver,  but they maybe can also be
> optimized.

My point is that your system *isn't* broken, because the DAC addresses
work fine, so there's really no need for it to spend any time messing
about with SAC constraints at all. Please try my patch[1].

Thanks,
Robin.

[1]
https://lore.kernel.org/linux-iommu/2b0ca6254dd0102bf559b2a73e9b51da089afbe3.1663764627.git.robin.murphy@arm.com/

>
>>
>> Since this mechanism now officially serves as a bandage for broken
>> hardware/drivers and little else, I had an idea to make it a lot
>> better, guess it's time to have a go at writing that patch up...
>>
>> Thanks,
>> Robin.
>>
>>> Signed-off-by: Chunguang Xu <chunguang.xu@shopee.com>
>>> ---
>>>   drivers/iommu/iova.c | 28 +++++++++++++++++++++-------
>>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>>> index 47d1983dfa2a..d5775719a143 100644
>>> --- a/drivers/iommu/iova.c
>>> +++ b/drivers/iommu/iova.c
>>> @@ -16,6 +16,7 @@
>>>   #define IOVA_ANCHOR    ~0UL
>>>   #define IOVA_RANGE_CACHE_MAX_SIZE 6    /* log of max cached IOVA
>>> range size (in pages) */
>>> +#define IOVA_RANGE_CACHE_ARRAY_SIZE (2 * IOVA_RANGE_CACHE_MAX_SIZE)
>>>   static bool iova_rcache_insert(struct iova_domain *iovad,
>>>                      unsigned long pfn,
>>> @@ -723,13 +724,13 @@ int iova_domain_init_rcaches(struct iova_domain
>>> *iovad)
>>>       unsigned int cpu;
>>>       int i, ret;
>>> -    iovad->rcaches = kcalloc(IOVA_RANGE_CACHE_MAX_SIZE,
>>> +    iovad->rcaches = kcalloc(IOVA_RANGE_CACHE_ARRAY_SIZE,
>>>                    sizeof(struct iova_rcache),
>>>                    GFP_KERNEL);
>>>       if (!iovad->rcaches)
>>>           return -ENOMEM;
>>> -    for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>>> +    for (i = 0; i < IOVA_RANGE_CACHE_ARRAY_SIZE; ++i) {
>>>           struct iova_cpu_rcache *cpu_rcache;
>>>           struct iova_rcache *rcache;
>>> @@ -825,11 +826,15 @@ static bool iova_rcache_insert(struct
>>> iova_domain *iovad, unsigned long pfn,
>>>                      unsigned long size)
>>>   {
>>>       unsigned int log_size = order_base_2(size);
>>> +    unsigned int index;
>>>       if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE)
>>>           return false;
>>> -    return __iova_rcache_insert(iovad, &iovad->rcaches[log_size], pfn);
>>> +    if (pfn > DMA_BIT_MASK(32))
>>> +        index = log_size + IOVA_RANGE_CACHE_MAX_SIZE;
>>> +
>>> +    return __iova_rcache_insert(iovad, &iovad->rcaches[index], pfn);
>>>   }
>>>   /*
>>> @@ -881,11 +886,20 @@ static unsigned long iova_rcache_get(struct
>>> iova_domain *iovad,
>>>                        unsigned long limit_pfn)
>>>   {
>>>       unsigned int log_size = order_base_2(size);
>>> +    unsigned long iova_pfn;
>>> +    unsigned int index;
>>>       if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE || !iovad->rcaches)
>>>           return 0;
>>> -    return __iova_rcache_get(&iovad->rcaches[log_size], limit_pfn -
>>> size);
>>> +    iova_pfn = __iova_rcache_get(&iovad->rcaches[log_size],
>>> limit_pfn - size);
>>> +
>>> +    if (!iova_pfn && limit_pfn > DMA_BIT_MASK(32)) {
>>> +        index = log_size + IOVA_RANGE_CACHE_MAX_SIZE;
>>> +        iova_pfn = __iova_rcache_get(&iovad->rcaches[index],
>>> limit_pfn - size);
>>> +    }
>>> +
>>> +    return iova_pfn
>>>   }
>>>   /*
>>> @@ -898,7 +912,7 @@ static void free_iova_rcaches(struct iova_domain
>>> *iovad)
>>>       unsigned int cpu;
>>>       int i, j;
>>> -    for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>>> +    for (i = 0; i < IOVA_RANGE_CACHE_ARRAY_SIZE; ++i) {
>>>           rcache = &iovad->rcaches[i];
>>>           if (!rcache->cpu_rcaches)
>>>               break;
>>> @@ -926,7 +940,7 @@ static void free_cpu_cached_iovas(unsigned int
>>> cpu, struct iova_domain *iovad)
>>>       unsigned long flags;
>>>       int i;
>>> -    for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>>> +    for (i = 0; i < IOVA_RANGE_CACHE_ARRAY_SIZE; ++i) {
>>>           rcache = &iovad->rcaches[i];
>>>           cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
>>>           spin_lock_irqsave(&cpu_rcache->lock, flags);
>>> @@ -945,7 +959,7 @@ static void free_global_cached_iovas(struct
>>> iova_domain *iovad)
>>>       unsigned long flags;
>>>       int i, j;
>>> -    for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>>> +    for (i = 0; i < IOVA_RANGE_CACHE_ARRAY_SIZE; ++i) {
>>>           rcache = &iovad->rcaches[i];
>>>           spin_lock_irqsave(&rcache->lock, flags);
>>>           for (j = 0; j < rcache->depot_size; ++j) {

\
 
 \ /
  Last update: 2022-09-23 14:32    [W:0.052 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site