linux-kernel - Re: [PATCH v3 1/2] iommu: Disable preemption around use of this_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 26 Jun 2016 12:54:19 +0200
From:	Thorsten Leemhuis <regressions@...mhuis.info>
To:	Joerg Roedel <joro@...tes.org>,
	Chris Wilson <chris@...is-wilson.co.uk>
Cc:	intel-gfx@...ts.freedesktop.org,
	Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
	iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] iommu: Disable preemption around use of
 this_cpu_ptr()

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