From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751147AbcEaKMG (ORCPT ); Tue, 31 May 2016 06:12:06 -0400 Received: from smtp.citrix.com ([66.165.176.89]:57509 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750740AbcEaKMB (ORCPT ); Tue, 31 May 2016 06:12:01 -0400 X-IronPort-AV: E=Sophos;i="5.26,395,1459814400"; d="scan'208";a="357487542" Subject: Re: [PATCH] xen: xen-pciback: Remove create_workqueue To: Konrad Rzeszutek Wilk , Tejun Heo References: <20160527155411.GA18039@Karyakshetra> <20160527160114.GB1842@char.us.oracle.com> <20160527160822.GO23194@mtj.duckdns.org> <20160527163201.GB2975@char.us.oracle.com> CC: Bhaktipriya Shridhar , , , , , , , , From: David Vrabel Message-ID: <574D636D.9010104@citrix.com> Date: Tue, 31 May 2016 11:11:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160527163201.GB2975@char.us.oracle.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-DLP: MIA2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27/05/16 17:32, Konrad Rzeszutek Wilk wrote: > On Fri, May 27, 2016 at 12:08:22PM -0400, Tejun Heo wrote: >> Hello, >> >> On Fri, May 27, 2016 at 12:01:14PM -0400, Konrad Rzeszutek Wilk wrote: >>> On Fri, May 27, 2016 at 09:24:11PM +0530, Bhaktipriya Shridhar wrote: >>>> With concurrency managed workqueues, use of dedicated workqueues can be >>>> replaced by using system_wq. Drop host->intr_wq by using >> ^ >> xen_pcibk_wq >>>> system_wq. >>>> >>>> Since there is only a single work item, increase of concurrency level by >>>> switching to system_wq should not break anything. >>> >>> _should_ not? Hehe. I presume this has not been tested? >> >> Yeah, this is a part of sweeping conversions and it's challenging (and >> often impossible for specific drivers) to setup test environments. >> xen isn't as bad but can still be a pretty specialized setup. The >> conversions aren't high risk and shouldn't be too difficult to root >> cause when something goes south. We'd greatly appreciate any helps >> with reviewing and testing. >> >>>> cancel_work_sync() has been used in xen_pcibk_disconnect() to ensure that >>>> work item is not pending or executing by the time exit path runs. >>>> >>>> Signed-off-by: Bhaktipriya Shridhar >>>> @@ -76,8 +75,7 @@ static void xen_pcibk_disconnect(struct xen_pcibk_device *pdev) >>>> /* If the driver domain started an op, make sure we complete it >>>> * before releasing the shared memory */ >>>> >>>> - /* Note, the workqueue does not use spinlocks at all.*/ >>>> - flush_workqueue(xen_pcibk_wq); >>>> + cancel_work_sync(&pdev->op_work); >> >> Should it be flush_work() instead? Is it okay for a pdev->op_work to >> be queued and canceled without actually getting executed? > > It should really flush them. The comment above says so, but in reality it > does not matter that much as we tearing down the communication. As long as > the pdev->op_work either completes or is never executed - we are fine. The comment should be updated to reflect this. David