Re: [PATCH v3 1/2] iommu: Disable preemption around use of this_cpu_ptr()

From: Thorsten Leemhuis
Date: Sun Jun 26 2016 - 06:54:26 EST


On 15.06.2016 14:25, Joerg Roedel wrote:
> On Wed, Jun 01, 2016 at 12:10:08PM +0100, Chris Wilson wrote:
>> Between acquiring the this_cpu_ptr() and using it, ideally we don't want
>> to be preempted and work on another CPU's private data. this_cpu_ptr()
>> checks whether or not preemption is disable, and get_cpu_ptr() provides
>> a convenient wrapper for operating on the cpu ptr inside a preemption
>> disabled critical section (which currently is provided by the
>> spinlock). Indeed if we disable preemption around this_cpu_ptr,
>> we do not need the CPU local spinlock - so long as take care that no other
>> CPU is running that code as do perform the cross-CPU cache flushing and
>> teardown, but that is a subject for another patch.
> [â]
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96293
> [â]
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index ba764a0835d3..e23001bfcfee 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -420,8 +420,10 @@ retry:
>>
>> /* Try replenishing IOVAs by flushing rcache. */
>> flushed_rcache = true;
>> + preempt_disable();
>> for_each_online_cpu(cpu)
>> free_cpu_cached_iovas(cpu, iovad);
>> + preempt_enable();
>
> Why do you need to disable preemption here? The free_cpu_cached_iovas
> function does not need to stay on the same cpu as it iterates over the
> rcaches for all cpus anyway.

Joerg, what's the status here? This made it on my 4.7 regressions
report, as the patches from this thread are supposed to fix a
regression; see
http://thread.gmane.org/gmane.linux.usb.general/143504/focus=153154
for details.

Please let me know if if fixes went to mainline already; I did a quick
check and could see any.

Sincerely, your regression tracker for Linux 4.7 (http://bit.ly/28JRmJo)
Thorsten