All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] xen: xen-pciback: Remove create_workqueue
@ 2016-06-01 14:15 Bhaktipriya Shridhar
  2016-06-01 14:53 ` David Vrabel
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Bhaktipriya Shridhar @ 2016-06-01 14:15 UTC (permalink / raw)
  To: Boris Ostrovsky, David Vrabel, Juergen Gross,
	Konrad Rzeszutek Wilk, Bhaktipriya Shridhar, Jan Beulich,
	Stefano Stabellini, Paul Gortmaker, Doug Goldstein
  Cc: Tejun Heo, xen-devel, linux-kernel

System workqueues have been able to handle high level of concurrency
for a long time now and there's no reason to use dedicated workqueues
just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
use of system_wq.

Unlike a dedicated per-cpu workqueue created with create_workqueue(),
system_wq allows multiple work items to overlap executions even on
the same CPU; however, a per-cpu workqueue doesn't have any CPU
locality or global ordering guarantees unless the target CPU is
explicitly specified and thus the increase of local concurrency shouldn't
make any difference.

Since the work items could be pending, flush_work() has been used in
xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
there is no pending task while disconnecting the driver.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 Changes in v2:
	-Changed cancel_work_sync to flush_work
	-Changed commit description

 Feedback to update a comment was received in v1 from David Vrabel. It has not
 be included in v2 since some clarification was required. Will include it in
 v3 once the details about the content and the placement of the comment are
 received.

 drivers/xen/xen-pciback/pciback.h     |  1 -
 drivers/xen/xen-pciback/pciback_ops.c |  2 +-
 drivers/xen/xen-pciback/xenbus.c      | 10 +---------
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
index 4d529f3..7af369b6 100644
--- a/drivers/xen/xen-pciback/pciback.h
+++ b/drivers/xen/xen-pciback/pciback.h
@@ -55,7 +55,6 @@ struct xen_pcibk_dev_data {

 /* Used by XenBus and xen_pcibk_ops.c */
 extern wait_queue_head_t xen_pcibk_aer_wait_queue;
-extern struct workqueue_struct *xen_pcibk_wq;
 /* Used by pcistub.c and conf_space_quirks.c */
 extern struct list_head xen_pcibk_quirks;

diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index 2f19dd7..f8c7775 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -310,7 +310,7 @@ void xen_pcibk_test_and_schedule_op(struct xen_pcibk_device *pdev)
 	 * already processing a request */
 	if (test_bit(_XEN_PCIF_active, (unsigned long *)&pdev->sh_info->flags)
 	    && !test_and_set_bit(_PDEVF_op_active, &pdev->flags)) {
-		queue_work(xen_pcibk_wq, &pdev->op_work);
+		schedule_work(&pdev->op_work);
 	}
 	/*_XEN_PCIB_active should have been cleared by pcifront. And also make
 	sure xen_pcibk is waiting for ack by checking _PCIB_op_pending*/
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index c252eb3..5ce878c 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -17,7 +17,6 @@
 #include "pciback.h"

 #define INVALID_EVTCHN_IRQ  (-1)
-struct workqueue_struct *xen_pcibk_wq;

 static bool __read_mostly passthrough;
 module_param(passthrough, bool, S_IRUGO);
@@ -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);
+	flush_work(&pdev->op_work);

 	if (pdev->sh_info != NULL) {
 		xenbus_unmap_ring_vfree(pdev->xdev, pdev->sh_info);
@@ -733,11 +731,6 @@ const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;

 int __init xen_pcibk_xenbus_register(void)
 {
-	xen_pcibk_wq = create_workqueue("xen_pciback_workqueue");
-	if (!xen_pcibk_wq) {
-		pr_err("%s: create xen_pciback_workqueue failed\n", __func__);
-		return -EFAULT;
-	}
 	xen_pcibk_backend = &xen_pcibk_vpci_backend;
 	if (passthrough)
 		xen_pcibk_backend = &xen_pcibk_passthrough_backend;
@@ -747,6 +740,5 @@ int __init xen_pcibk_xenbus_register(void)

 void __exit xen_pcibk_xenbus_unregister(void)
 {
-	destroy_workqueue(xen_pcibk_wq);
 	xenbus_unregister_driver(&xen_pcibk_driver);
 }
--
2.1.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
  2016-06-01 14:53 ` David Vrabel
@ 2016-06-01 14:53 ` David Vrabel
  2016-06-01 15:45 ` Tejun Heo
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2016-06-01 14:53 UTC (permalink / raw)
  To: Bhaktipriya Shridhar, Boris Ostrovsky, David Vrabel,
	Juergen Gross, Konrad Rzeszutek Wilk, Jan Beulich,
	Stefano Stabellini, Paul Gortmaker, Doug Goldstein
  Cc: Tejun Heo, xen-devel, linux-kernel

On 01/06/16 15:15, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	-Changed cancel_work_sync to flush_work
> 	-Changed commit description
> 
>  Feedback to update a comment was received in v1 from David Vrabel. It has not
>  be included in v2 since some clarification was required. Will include it in
>  v3 once the details about the content and the placement of the comment are
>  received.

The comment needed updating iff this continued to use cancel_work_sync()
but since it's using flush_work() the comment is fine.

David

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
@ 2016-06-01 14:53 ` David Vrabel
  2016-06-01 14:53 ` [Xen-devel] " David Vrabel
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2016-06-01 14:53 UTC (permalink / raw)
  To: Bhaktipriya Shridhar, Boris Ostrovsky, David Vrabel,
	Juergen Gross, Konrad Rzeszutek Wilk, Jan Beulich,
	Stefano Stabellini, Paul Gortmaker, Doug Goldstein
  Cc: Tejun Heo, xen-devel, linux-kernel

On 01/06/16 15:15, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	-Changed cancel_work_sync to flush_work
> 	-Changed commit description
> 
>  Feedback to update a comment was received in v1 from David Vrabel. It has not
>  be included in v2 since some clarification was required. Will include it in
>  v3 once the details about the content and the placement of the comment are
>  received.

The comment needed updating iff this continued to use cancel_work_sync()
but since it's using flush_work() the comment is fine.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
                   ` (2 preceding siblings ...)
  2016-06-01 15:45 ` Tejun Heo
@ 2016-06-01 15:45 ` Tejun Heo
  2016-06-28 16:46   ` Bhaktipriya Shridhar
  2016-06-28 16:46   ` Bhaktipriya Shridhar
  2016-06-29 12:49 ` [Xen-devel] " David Vrabel
  2016-06-29 12:49 ` David Vrabel
  5 siblings, 2 replies; 9+ messages in thread
From: Tejun Heo @ 2016-06-01 15:45 UTC (permalink / raw)
  To: Bhaktipriya Shridhar
  Cc: Boris Ostrovsky, David Vrabel, Juergen Gross,
	Konrad Rzeszutek Wilk, Jan Beulich, Stefano Stabellini,
	Paul Gortmaker, Doug Goldstein, xen-devel, linux-kernel

On Wed, Jun 01, 2016 at 07:45:08PM +0530, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
  2016-06-01 14:53 ` David Vrabel
  2016-06-01 14:53 ` [Xen-devel] " David Vrabel
@ 2016-06-01 15:45 ` Tejun Heo
  2016-06-01 15:45 ` Tejun Heo
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2016-06-01 15:45 UTC (permalink / raw)
  To: Bhaktipriya Shridhar
  Cc: Juergen Gross, Stefano Stabellini, Doug Goldstein, linux-kernel,
	Paul Gortmaker, David Vrabel, Jan Beulich, xen-devel,
	Boris Ostrovsky

On Wed, Jun 01, 2016 at 07:45:08PM +0530, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 15:45 ` Tejun Heo
@ 2016-06-28 16:46   ` Bhaktipriya Shridhar
  2016-06-28 16:46   ` Bhaktipriya Shridhar
  1 sibling, 0 replies; 9+ messages in thread
From: Bhaktipriya Shridhar @ 2016-06-28 16:46 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Boris Ostrovsky, David Vrabel, Juergen Gross,
	Konrad Rzeszutek Wilk, Jan Beulich, Stefano Stabellini,
	Paul Gortmaker, Doug Goldstein, xen-devel,
	Linux-Kernel@Vger. Kernel. Org

Ping!
Thanks,
Bhaktipriya


On Wed, Jun 1, 2016 at 9:15 PM, Tejun Heo <tj@kernel.org> wrote:
> On Wed, Jun 01, 2016 at 07:45:08PM +0530, Bhaktipriya Shridhar wrote:
>> System workqueues have been able to handle high level of concurrency
>> for a long time now and there's no reason to use dedicated workqueues
>> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
>> use of system_wq.
>>
>> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
>> system_wq allows multiple work items to overlap executions even on
>> the same CPU; however, a per-cpu workqueue doesn't have any CPU
>> locality or global ordering guarantees unless the target CPU is
>> explicitly specified and thus the increase of local concurrency shouldn't
>> make any difference.
>>
>> Since the work items could be pending, flush_work() has been used in
>> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
>> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
>> there is no pending task while disconnecting the driver.
>>
>> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> Thanks.
>
> --
> tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 15:45 ` Tejun Heo
  2016-06-28 16:46   ` Bhaktipriya Shridhar
@ 2016-06-28 16:46   ` Bhaktipriya Shridhar
  1 sibling, 0 replies; 9+ messages in thread
From: Bhaktipriya Shridhar @ 2016-06-28 16:46 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Juergen Gross, Stefano Stabellini, Doug Goldstein,
	Linux-Kernel@Vger. Kernel. Org, Paul Gortmaker, David Vrabel,
	Jan Beulich, xen-devel, Boris Ostrovsky

Ping!
Thanks,
Bhaktipriya


On Wed, Jun 1, 2016 at 9:15 PM, Tejun Heo <tj@kernel.org> wrote:
> On Wed, Jun 01, 2016 at 07:45:08PM +0530, Bhaktipriya Shridhar wrote:
>> System workqueues have been able to handle high level of concurrency
>> for a long time now and there's no reason to use dedicated workqueues
>> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
>> use of system_wq.
>>
>> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
>> system_wq allows multiple work items to overlap executions even on
>> the same CPU; however, a per-cpu workqueue doesn't have any CPU
>> locality or global ordering guarantees unless the target CPU is
>> explicitly specified and thus the increase of local concurrency shouldn't
>> make any difference.
>>
>> Since the work items could be pending, flush_work() has been used in
>> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
>> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
>> there is no pending task while disconnecting the driver.
>>
>> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> Thanks.
>
> --
> tejun

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
                   ` (3 preceding siblings ...)
  2016-06-01 15:45 ` Tejun Heo
@ 2016-06-29 12:49 ` David Vrabel
  2016-06-29 12:49 ` David Vrabel
  5 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2016-06-29 12:49 UTC (permalink / raw)
  To: Bhaktipriya Shridhar, Boris Ostrovsky, David Vrabel,
	Juergen Gross, Konrad Rzeszutek Wilk, Jan Beulich,
	Stefano Stabellini, Paul Gortmaker, Doug Goldstein
  Cc: Tejun Heo, xen-devel, linux-kernel

On 01/06/16 15:15, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.

Applied to for-linus-4.8, thanks.

David

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xen: xen-pciback: Remove create_workqueue
  2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
                   ` (4 preceding siblings ...)
  2016-06-29 12:49 ` [Xen-devel] " David Vrabel
@ 2016-06-29 12:49 ` David Vrabel
  5 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2016-06-29 12:49 UTC (permalink / raw)
  To: Bhaktipriya Shridhar, Boris Ostrovsky, David Vrabel,
	Juergen Gross, Konrad Rzeszutek Wilk, Jan Beulich,
	Stefano Stabellini, Paul Gortmaker, Doug Goldstein
  Cc: Tejun Heo, xen-devel, linux-kernel

On 01/06/16 15:15, Bhaktipriya Shridhar wrote:
> System workqueues have been able to handle high level of concurrency
> for a long time now and there's no reason to use dedicated workqueues
> just to gain concurrency.  Replace dedicated xen_pcibk_wq with the
> use of system_wq.
> 
> Unlike a dedicated per-cpu workqueue created with create_workqueue(),
> system_wq allows multiple work items to overlap executions even on
> the same CPU; however, a per-cpu workqueue doesn't have any CPU
> locality or global ordering guarantees unless the target CPU is
> explicitly specified and thus the increase of local concurrency shouldn't
> make any difference.
> 
> Since the work items could be pending, flush_work() has been used in
> xen_pcibk_disconnect(). xen_pcibk_xenbus_remove() calls free_pdev()
> which in turn calls xen_pcibk_disconnect() for every pdev to ensure that
> there is no pending task while disconnecting the driver.

Applied to for-linus-4.8, thanks.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-29 12:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 14:15 [PATCH v2] xen: xen-pciback: Remove create_workqueue Bhaktipriya Shridhar
2016-06-01 14:53 ` David Vrabel
2016-06-01 14:53 ` [Xen-devel] " David Vrabel
2016-06-01 15:45 ` Tejun Heo
2016-06-01 15:45 ` Tejun Heo
2016-06-28 16:46   ` Bhaktipriya Shridhar
2016-06-28 16:46   ` Bhaktipriya Shridhar
2016-06-29 12:49 ` [Xen-devel] " David Vrabel
2016-06-29 12:49 ` David Vrabel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.