lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Patch v3 2/3] iommu: optimize iova_magazine_free_pfns()
From
Date
On 21/01/2020 5:29 pm, Cong Wang wrote:
> On Tue, Jan 21, 2020 at 1:52 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 18/12/2019 4:39 am, Cong Wang wrote:
>>> If the magazine is empty, iova_magazine_free_pfns() should
>>> be a nop, however it misses the case of mag->size==0. So we
>>> should just call iova_magazine_empty().
>>>
>>> This should reduce the contention on iovad->iova_rbtree_lock
>>> a little bit, not much at all.
>>
>> Have you measured that in any way? AFAICS the only time this can get
>> called with a non-full magazine is in the CPU hotplug callback, where
>> the impact of taking the rbtree lock and immediately releasing it seems
>> unlikely to be significant on top of everything else involved in that
>> operation.
>
> This patchset is only tested as a whole, it is not easy to deploy
> each to production and test it separately.
>
> Is there anything wrong to optimize a CPU hotplug path? :) And,
> it is called in alloc_iova_fast() too when, for example, over-cached.

And if the IOVA space is consumed to the point that we've fallen back to
that desperate last resort, what do you think the chances are that a
significant number of percpu magazines will be *empty*? Also bear in
mind that in that case we've already walked the rbtree once, so any
notion of still being fast is long, long gone.

As for CPU hotplug, it's a comparatively rare event involving all manner
of system-wide synchronisation, and the "optimisation" of shaving a few
dozen CPU cycles off at one point *if* things happen to line up
correctly is taking a cup of water out of a lake. If the domain is busy
at the time, then once again chances are the magazines aren't empty and
having an extra check redundant with the loop condition simply adds
(trivial, but nonzero) overhead to every call. And if the domain isn't
busy, then the lock is unlikely to be contended anyway.

Sorry, but without convincing evidence, this change just looks like
churn for the sake of it.

Robin.

\
 
 \ /
  Last update: 2020-01-22 18:34    [W:0.105 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site