Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1408177

Re: [PATCH] xen: xen-pciback: Remove create_workqueue

From Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Newsgroups linux.kernel
Subject Re: [PATCH] xen: xen-pciback: Remove create_workqueue
Date 2016-05-27 18:40 +0200
Message-ID <rDsXM-5NX-41@gated-at.bofh.it> (permalink)
References <rDsl4-5l0-13@gated-at.bofh.it> <rDsuJ-5E6-5@gated-at.bofh.it> <rDsuJ-5E6-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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 <bhaktipriya96@gmail.com>
> > > @@ -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.

> 
> Thanks.
> 
> -- 
> tejun

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar <bhaktipriya96@gmail.com> - 2016-05-27 18:00 +0200
  Re: [PATCH] xen: xen-pciback: Remove create_workqueue Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-05-27 18:10 +0200
    Re: [PATCH] xen: xen-pciback: Remove create_workqueue Tejun Heo <tj@kernel.org> - 2016-05-27 18:10 +0200
      Re: [PATCH] xen: xen-pciback: Remove create_workqueue Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-05-27 18:40 +0200

csiph-web