Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp358428pxu; Wed, 14 Oct 2020 03:19:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzsWXtfM2LhOg776P8ofJ3hyas8ZvCtotAK9Bd62GQVjSnTfTsMV9tn2CkKdgHyjD6Djhbk X-Received: by 2002:a17:906:3751:: with SMTP id e17mr4340876ejc.241.1602670751097; Wed, 14 Oct 2020 03:19:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1602670751; cv=none; d=google.com; s=arc-20160816; b=rWtbTaHozBf8WYhoCa60QarcxoircH/VlxZHhNOFX5Y/hjqF+b8CFGrFZT1WC9EXcX oX1dNd7g5VZjObXZaGb9BikVkVIrVXtu9CU8h0gA31IkjP7N6KF0qHYSGvHGlpL6I99M eyu+VDT9vBromg9xunZU4kME3BNjGUHa2OY1MgQPTnUei2ln6//CZY4HC3s/uc4ytRa6 fVT+ZnDxoFBHFXPSFkvyij0gwML9KtFDXkuA6vAmOMtT3l3yPq6C1JTmvHVcLEPiDTlK HuqzMzCFJJxnRj+PPXfebfB7BFtU93/8ug0Wl3mFBb/pYPCzRs9bLOBwKFp1vg0M8ikK BsMw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:content-transfer-encoding :content-language:accept-language:in-reply-to:references:message-id :date:thread-index:thread-topic:subject:cc:to:from; bh=4J/w5E3KC7jlZqWJRPCPNvMhqWvDOxDSYAAPpeweMLg=; b=GMCt5XL59cJDrjYGnpH45mZE5SVMP3sH7S35eQIQAXuyUrqb8eLITBKs47mvAB6Exv 91Boou2D2omBiMUx5zZixmb6IsBZVOq/I8iqni5zMivTD1PPiZCGnqWfwpCeyGPiYhBT 1ofpE8FDRK0n2qrpyhXBGbES+9RHY0LJs4ZXdnJPicjDTEWavYvpGUDB8vuXzqw8/JGf 7qSp5wINijKbe6y3dKJ/3bPPOBoHW4sZaq8jsOVO1ujZk4zjOdhO1Kjt5W3XDNjmXIOV az+OyDE3T1yvuRBnA4Mo+Mp6i9ETz9joRENFq0aBanKF7Z+M/hKh8tUFQNsi7IaMDAy9 RNzw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id p19si1803854eji.609.2020.10.14.03.18.48; Wed, 14 Oct 2020 03:19:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387915AbgJNBTk convert rfc822-to-8bit (ORCPT + 99 others); Tue, 13 Oct 2020 21:19:40 -0400 Received: from szxga02-in.huawei.com ([45.249.212.188]:3570 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730908AbgJNBTk (ORCPT ); Tue, 13 Oct 2020 21:19:40 -0400 Received: from dggeme759-chm.china.huawei.com (unknown [172.30.72.53]) by Forcepoint Email with ESMTP id 467F449E946C95D872EE; Wed, 14 Oct 2020 09:19:37 +0800 (CST) Received: from dggemi761-chm.china.huawei.com (10.1.198.147) by dggeme759-chm.china.huawei.com (10.3.19.105) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1913.5; Wed, 14 Oct 2020 09:19:36 +0800 Received: from dggemi761-chm.china.huawei.com ([10.9.49.202]) by dggemi761-chm.china.huawei.com ([10.9.49.202]) with mapi id 15.01.1913.007; Wed, 14 Oct 2020 09:19:36 +0800 From: "Song Bao Hua (Barry Song)" To: Alex Williamson CC: "tiantao (H)" , "eric.auger@redhat.com" , "cohuck@redhat.com" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Linuxarm Subject: RE: [PATCH] vfio/platform: Replace spin_lock_irqsave by spin_lock in hard IRQ Thread-Topic: [PATCH] vfio/platform: Replace spin_lock_irqsave by spin_lock in hard IRQ Thread-Index: AQHWoQSZTGOwJR7vqkiLltZ8f3MgN6mViI6AgACwRXD//4cGgIAAh/NA Date: Wed, 14 Oct 2020 01:19:36 +0000 Message-ID: <6dfbaccf24514624b56cb47ea975f4a7@hisilicon.com> References: <1602554458-26927-1-git-send-email-tiantao6@hisilicon.com> <20201013153229.7fe74e65@w520.home> <20201013185023.455a6ca9@x1.home> In-Reply-To: <20201013185023.455a6ca9@x1.home> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.126.200.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Wednesday, October 14, 2020 1:50 PM > To: Song Bao Hua (Barry Song) > Cc: tiantao (H) ; eric.auger@redhat.com; > cohuck@redhat.com; kvm@vger.kernel.org; linux-kernel@vger.kernel.org; > Linuxarm > Subject: Re: [PATCH] vfio/platform: Replace spin_lock_irqsave by spin_lock in > hard IRQ > > On Wed, 14 Oct 2020 00:15:13 +0000 > "Song Bao Hua (Barry Song)" wrote: > > > > -----Original Message----- > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > Sent: Wednesday, October 14, 2020 10:32 AM > > > To: tiantao (H) > > > Cc: eric.auger@redhat.com; cohuck@redhat.com; kvm@vger.kernel.org; > > > linux-kernel@vger.kernel.org; Song Bao Hua (Barry Song) > > > ; Linuxarm > > > Subject: Re: [PATCH] vfio/platform: Replace spin_lock_irqsave by spin_lock > in > > > hard IRQ > > > > > > On Tue, 13 Oct 2020 10:00:58 +0800 > > > Tian Tao wrote: > > > > > > > It is redundant to do irqsave and irqrestore in hardIRQ context. > > > > > > But this function is also called from non-IRQ context. Thanks, > > > > It seems you mean > > vfio_platform_set_irqs_ioctl() -> > > vfio_platform_set_irq_trigger -> > > handler() ? > > Yes. > > > so, will it be better to move the irqsave out of the > vfio_automasked_irq_handler() > > and put it to where the function is called in non-IRQ context? > > > > I mean: > > > > irqhandler() > > { > > spin_lock() //without irqsave > > spin_unlock() > > } > > > > Non-irq context which is calling this handler: > > irqsave(); > > irqhandler(); > > irqrestore(); > > > > Anyway, if it is called in IRQ context, it is redundant to do irqsave. > > What's the advantage? You're saying it's redundant, is it also wrong? It is not wrong and it doesn't make any malfunction. It just takes a couple of instruction cycles to do save/restore and irq-disable/enable of cpu, which is useless in irq context. So the advantage is that we are going to remove some redundant instruction cycles. And if the irq handler is called very often, we speed up the system. > If it's not wrong and only redundant, what's the tangible latency > difference in maintaining a separate IRQ context handler without the > irqsave/restore? Thanks, For this question, maybe need some benchmark to get answer. If the irqhandler is not called that often, I agree it might be not worth to maintain two pieces of code. > > Alex > > > > > Signed-off-by: Tian Tao > > > > --- > > > > drivers/vfio/platform/vfio_platform_irq.c | 5 ++--- > > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/vfio/platform/vfio_platform_irq.c > > > b/drivers/vfio/platform/vfio_platform_irq.c > > > > index c5b09ec..24fd6c5 100644 > > > > --- a/drivers/vfio/platform/vfio_platform_irq.c > > > > +++ b/drivers/vfio/platform/vfio_platform_irq.c > > > > @@ -139,10 +139,9 @@ static int vfio_platform_set_irq_unmask(struct > > > vfio_platform_device *vdev, > > > > static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id) > > > > { > > > > struct vfio_platform_irq *irq_ctx = dev_id; > > > > - unsigned long flags; > > > > int ret = IRQ_NONE; > > > > > > > > - spin_lock_irqsave(&irq_ctx->lock, flags); > > > > + spin_lock(&irq_ctx->lock); > > > > > > > > if (!irq_ctx->masked) { > > > > ret = IRQ_HANDLED; > > > > @@ -152,7 +151,7 @@ static irqreturn_t > vfio_automasked_irq_handler(int > > > irq, void *dev_id) > > > > irq_ctx->masked = true; > > > > } > > > > > > > > - spin_unlock_irqrestore(&irq_ctx->lock, flags); > > > > + spin_unlock(&irq_ctx->lock); > > > > > > > > if (ret == IRQ_HANDLED) > > > > eventfd_signal(irq_ctx->trigger, 1); > > Thanks Barry