From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751951AbdKFUH2 convert rfc822-to-8bit (ORCPT ); Mon, 6 Nov 2017 15:07:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59604 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751023AbdKFUH1 (ORCPT ); Mon, 6 Nov 2017 15:07:27 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D3FAC80F9C Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=alex.williamson@redhat.com Date: Mon, 6 Nov 2017 13:07:26 -0700 From: Alex Williamson To: Robin Murphy Cc: Sebastian Andrzej Siewior , Joerg Roedel , iommu@lists.linux-foundation.org, vinadhy@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] iommu/iova: Use raw_cpu_ptr() instead of get_cpu_ptr() for ->fq Message-ID: <20171106130726.0685da5b@t450s.home> In-Reply-To: <5653117e-7708-de28-f349-ffe5ebf4481d@arm.com> References: <20170906103459.oi2nn7jondjqdo5m@linutronix.de> <20170919094119.bibxslqyeefbx7gf@8bytes.org> <20170921152140.grayy3a7ok2im7xa@linutronix.de> <20171102173349.btd3b2xfklcfopmz@breakpoint.cc> <20171106084334.639512ea@t450s.home> <5653117e-7708-de28-f349-ffe5ebf4481d@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 06 Nov 2017 20:07:27 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 6 Nov 2017 16:45:11 +0000 Robin Murphy wrote: > On 06/11/17 15:43, Alex Williamson wrote: > > [cc +robin] > > > > On Thu, 2 Nov 2017 18:33:50 +0100 > > Sebastian Andrzej Siewior wrote: > > > >> On 2017-09-21 17:21:40 [+0200], Sebastian Andrzej Siewior wrote: > >>> get_cpu_ptr() disabled preemption and returns the ->fq object of the > >>> current CPU. raw_cpu_ptr() does the same except that it not disable > >>> preemption which means the scheduler can move it to another CPU after it > >>> obtained the per-CPU object. > >>> In this case this is not bad because the data structure itself is > >>> protected with a spin_lock. This change shouldn't matter however on RT > >>> it does because the sleeping lock can't be accessed with disabled > >>> preemption. > >> > >> Did this make to your tree Jörg? > > > > Hi Sebastian, > > > > Joerg is out on paternity leave through the end of the year, I'm > > filling in in the interim. I hadn't looked for patches this far back, > > so thanks for pointing it out. Robin, any comments? Thanks, > > The reasoning seems sound - assuming we can't get preempted once the > lock is actually taken, the worst side-effect of getting moved to > another CPU in that slim window looks to be a little bit of lock > contention. Operating on "someone else's" queue should have no > correctness impact, and the cmpxchg after the lock is released isn't > even working on percpu data so doesn't really care anyway. > > AFAICS it's more or less the direct equivalent of aaffaa8a3b59 > ("iommu/iova: Don't disable preempt around this_cpu_ptr()"), which seems > to have been working out OK. Great, thanks Robin. Applied to iommu/iova for v4.15. Thanks, Alex > >>> Cc: Joerg Roedel > >>> Cc: iommu@lists.linux-foundation.org > >>> Reported-by: vinadhy@gmail.com > >>> Signed-off-by: Sebastian Andrzej Siewior > >>> --- > >>> On 2017-09-19 11:41:19 [+0200], Joerg Roedel wrote: > >>>> Hi Sebastian, > >>> Hi Jörg, > >>> > >>>> I moved the flushing to driver/iommu/iova.c to share it with the Intel > >>>> IOMMU and possibly other drivers too, so this patch does no longer apply > >>>> to v4.14-rc1. Can you update the patch to these changes? > >>> > >>> Sure. > >>> > >>> v1…v2: move the change from amd_iommu.c to iova.c > >>> > >>> drivers/iommu/iova.c | 4 +--- > >>> 1 file changed, 1 insertion(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c > >>> index 33edfa794ae9..b30900025c62 100644 > >>> --- a/drivers/iommu/iova.c > >>> +++ b/drivers/iommu/iova.c > >>> @@ -570,7 +570,7 @@ void queue_iova(struct iova_domain *iovad, > >>> unsigned long pfn, unsigned long pages, > >>> unsigned long data) > >>> { > >>> - struct iova_fq *fq = get_cpu_ptr(iovad->fq); > >>> + struct iova_fq *fq = raw_cpu_ptr(iovad->fq); > >>> unsigned long flags; > >>> unsigned idx; > >>> > >>> @@ -600,8 +600,6 @@ void queue_iova(struct iova_domain *iovad, > >>> if (atomic_cmpxchg(&iovad->fq_timer_on, 0, 1) == 0) > >>> mod_timer(&iovad->fq_timer, > >>> jiffies + msecs_to_jiffies(IOVA_FQ_TIMEOUT)); > >>> - > >>> - put_cpu_ptr(iovad->fq); > >>> } > >>> EXPORT_SYMBOL_GPL(queue_iova); > >>> > >>> -- > >>> 2.14.1 > >>> > >> > >> Sebastian > >> _______________________________________________ > >> iommu mailing list > >> iommu@lists.linux-foundation.org > >> https://lists.linuxfoundation.org/mailman/listinfo/iommu >