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;
Message-ID: <51af869a-83d4-631a-2d91-edb8b066bf4d@huawei.com>
Date:   Fri, 1 Jul 2022 12:33:17 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.1
Subject: Re: [PATCH] iommu/iova: change IOVA_MAG_SIZE to 127 to save memory
To:     Feng Tang <feng.tang@intel.com>
CC:     Robin Murphy <robin.murphy@arm.com>,
        Joerg Roedel <joro@8bytes.org>,
        "Will Deacon" <will@kernel.org>,
        <iommu@lists.linux-foundation.org>, <iommu@lists.linux.dev>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Christoph Lameter" <cl@linux.com>,
        Vlastimil Babka <vbabka@suse.cz>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>, Paul Menzel <pmenzel@molgen.mpg.de>
References: <20220630073304.26945-1-feng.tang@intel.com>
 <13db50bb-57c7-0d54-3857-84b8a4591d9e@arm.com>
 <7c29d01d-d90c-58d3-a6e0-0b6c404173ac@huawei.com>
 <117b31b5-8d06-0af4-7f1c-231d86becf1d@arm.com>
 <2920df89-9975-5785-f79b-257d3052dfaf@huawei.com>
 <20220701035622.GB14806@shbuild999.sh.intel.com>
From:   John Garry <john.garry@huawei.com>
In-Reply-To: <20220701035622.GB14806@shbuild999.sh.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 01/07/2022 04:56, Feng Tang wrote:
>>> inclination.
>>>
>> ok, what you are saying sounds reasonable. I just remember that when we
>> analyzed the longterm aging issue that we concluded that the FQ size and its
>> relation to the magazine size was a factor and this change makes me a little
>> worried about new issues. Better the devil you know and all that...
>>
>> Anyway, if I get some time I might do some testing to see if this change has
>> any influence.
>>
>> Another thought is if we need even store the size in the iova_magazine? mags
>> in the depot are always full. As such, we only need worry about mags loaded
>> in the cpu rcache and their sizes, so maybe we could have something like
>> this:
>>
>> struct iova_magazine {
>> -       unsigned long size;
>>         unsigned long pfns[IOVA_MAG_SIZE];
>> };
>>
>> @@ -631,6 +630,8 @@ struct iova_cpu_rcache {
>>         spinlock_t lock;
>>         struct iova_magazine *loaded;
>>         struct iova_magazine *prev;
>> +       int loaded_size;
>> +       int prev_size;
>> };
>>
>> I haven't tried to implement it though..
> I have very few knowledge of iova, so you can chose what's the better
> solution. I just wanted to raise the problem and will be happy to see
> it solved:)

I quickly tested your patch for performance and saw no noticeable 
difference, which is no surprise.

But I'll defer to Robin if he thinks that your patch is a better 
solution - I would guess that he does. For me personally I would prefer 
that this value was not changed, as I mentioned before.

thanks,
John