* [PATCH] connector: remove lazy workqueue creation
@ 2010-10-15 9:55 Tejun Heo
2010-10-15 13:06 ` Evgeniy Polyakov
2010-10-24 21:21 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Tejun Heo @ 2010-10-15 9:55 UTC (permalink / raw)
To: Evgeniy Polyakov, netdev@vger.kernel.org, Frederic Weisbecker,
David S. Miller
Commit 1a5645bc (connector: create connector workqueue only while
needed once) implements lazy workqueue creation for connector
workqueue. With cmwq now in place, lazy workqueue creation doesn't
make much sense while adding a lot of complexity. Remove it and
allocate an ordered workqueue during initialization.
This also removes a call to flush_scheduled_work() which is deprecated
and scheduled to be removed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
drivers/connector/cn_queue.c | 75 ++++--------------------------------------
drivers/connector/connector.c | 9 ++---
include/linux/connector.h | 8 ----
3 files changed, 12 insertions(+), 80 deletions(-)
Index: work/drivers/connector/cn_queue.c
===================================================================
--- work.orig/drivers/connector/cn_queue.c
+++ work/drivers/connector/cn_queue.c
@@ -31,48 +31,6 @@
#include <linux/connector.h>
#include <linux/delay.h>
-
-/*
- * This job is sent to the kevent workqueue.
- * While no event is once sent to any callback, the connector workqueue
- * is not created to avoid a useless waiting kernel task.
- * Once the first event is received, we create this dedicated workqueue which
- * is necessary because the flow of data can be high and we don't want
- * to encumber keventd with that.
- */
-static void cn_queue_create(struct work_struct *work)
-{
- struct cn_queue_dev *dev;
-
- dev = container_of(work, struct cn_queue_dev, wq_creation);
-
- dev->cn_queue = create_singlethread_workqueue(dev->name);
- /* If we fail, we will use keventd for all following connector jobs */
- WARN_ON(!dev->cn_queue);
-}
-
-/*
- * Queue a data sent to a callback.
- * If the connector workqueue is already created, we queue the job on it.
- * Otherwise, we queue the job to kevent and queue the connector workqueue
- * creation too.
- */
-int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work)
-{
- struct cn_queue_dev *pdev = cbq->pdev;
-
- if (likely(pdev->cn_queue))
- return queue_work(pdev->cn_queue, work);
-
- /* Don't create the connector workqueue twice */
- if (atomic_inc_return(&pdev->wq_requested) == 1)
- schedule_work(&pdev->wq_creation);
- else
- atomic_dec(&pdev->wq_requested);
-
- return schedule_work(work);
-}
-
void cn_queue_wrapper(struct work_struct *work)
{
struct cn_callback_entry *cbq =
@@ -111,11 +69,7 @@ cn_queue_alloc_callback_entry(char *name
static void cn_queue_free_callback(struct cn_callback_entry *cbq)
{
- /* The first jobs have been sent to kevent, flush them too */
- flush_scheduled_work();
- if (cbq->pdev->cn_queue)
- flush_workqueue(cbq->pdev->cn_queue);
-
+ flush_workqueue(cbq->pdev->cn_queue);
kfree(cbq);
}
@@ -193,11 +147,14 @@ struct cn_queue_dev *cn_queue_alloc_dev(
atomic_set(&dev->refcnt, 0);
INIT_LIST_HEAD(&dev->queue_list);
spin_lock_init(&dev->queue_lock);
- init_waitqueue_head(&dev->wq_created);
dev->nls = nls;
- INIT_WORK(&dev->wq_creation, cn_queue_create);
+ dev->cn_queue = alloc_ordered_workqueue(dev->name, 0);
+ if (!dev->cn_queue) {
+ kfree(dev);
+ return NULL;
+ }
return dev;
}
@@ -205,25 +162,9 @@ struct cn_queue_dev *cn_queue_alloc_dev(
void cn_queue_free_dev(struct cn_queue_dev *dev)
{
struct cn_callback_entry *cbq, *n;
- long timeout;
- DEFINE_WAIT(wait);
- /* Flush the first pending jobs queued on kevent */
- flush_scheduled_work();
-
- /* If the connector workqueue creation is still pending, wait for it */
- prepare_to_wait(&dev->wq_created, &wait, TASK_UNINTERRUPTIBLE);
- if (atomic_read(&dev->wq_requested) && !dev->cn_queue) {
- timeout = schedule_timeout(HZ * 2);
- if (!timeout && !dev->cn_queue)
- WARN_ON(1);
- }
- finish_wait(&dev->wq_created, &wait);
-
- if (dev->cn_queue) {
- flush_workqueue(dev->cn_queue);
- destroy_workqueue(dev->cn_queue);
- }
+ flush_workqueue(dev->cn_queue);
+ destroy_workqueue(dev->cn_queue);
spin_lock_bh(&dev->queue_lock);
list_for_each_entry_safe(cbq, n, &dev->queue_list, callback_entry)
Index: work/drivers/connector/connector.c
===================================================================
--- work.orig/drivers/connector/connector.c
+++ work/drivers/connector/connector.c
@@ -133,7 +133,8 @@ static int cn_call_callback(struct sk_bu
__cbq->data.skb == NULL)) {
__cbq->data.skb = skb;
- if (queue_cn_work(__cbq, &__cbq->work))
+ if (queue_work(dev->cbdev->cn_queue,
+ &__cbq->work))
err = 0;
else
err = -EINVAL;
@@ -148,13 +149,11 @@ static int cn_call_callback(struct sk_bu
d->callback = __cbq->data.callback;
d->free = __new_cbq;
- __new_cbq->pdev = __cbq->pdev;
-
INIT_WORK(&__new_cbq->work,
&cn_queue_wrapper);
- if (queue_cn_work(__new_cbq,
- &__new_cbq->work))
+ if (queue_work(dev->cbdev->cn_queue,
+ &__new_cbq->work))
err = 0;
else {
kfree(__new_cbq);
Index: work/include/linux/connector.h
===================================================================
--- work.orig/include/linux/connector.h
+++ work/include/linux/connector.h
@@ -88,12 +88,6 @@ struct cn_queue_dev {
unsigned char name[CN_CBQ_NAMELEN];
struct workqueue_struct *cn_queue;
- /* Sent to kevent to create cn_queue only when needed */
- struct work_struct wq_creation;
- /* Tell if the wq_creation job is pending/completed */
- atomic_t wq_requested;
- /* Wait for cn_queue to be created */
- wait_queue_head_t wq_created;
struct list_head queue_list;
spinlock_t queue_lock;
@@ -141,8 +135,6 @@ int cn_netlink_send(struct cn_msg *, u32
int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *, struct netlink_skb_parms *));
void cn_queue_del_callback(struct cn_queue_dev *dev, struct cb_id *id);
-int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work);
-
struct cn_queue_dev *cn_queue_alloc_dev(char *name, struct sock *);
void cn_queue_free_dev(struct cn_queue_dev *dev);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connector: remove lazy workqueue creation
2010-10-15 9:55 [PATCH] connector: remove lazy workqueue creation Tejun Heo
@ 2010-10-15 13:06 ` Evgeniy Polyakov
2010-10-15 13:38 ` Tejun Heo
2010-10-24 21:21 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Evgeniy Polyakov @ 2010-10-15 13:06 UTC (permalink / raw)
To: Tejun Heo; +Cc: netdev@vger.kernel.org, Frederic Weisbecker, David S. Miller
Hi Tejun.
On Fri, Oct 15, 2010 at 11:55:22AM +0200, Tejun Heo (tj@kernel.org) wrote:
> Commit 1a5645bc (connector: create connector workqueue only while
> needed once) implements lazy workqueue creation for connector
> workqueue. With cmwq now in place, lazy workqueue creation doesn't
> make much sense while adding a lot of complexity. Remove it and
> allocate an ordered workqueue during initialization.
>
> This also removes a call to flush_scheduled_work() which is deprecated
> and scheduled to be removed.
Looks good, thank you.
Ack.
--
Evgeniy Polyakov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connector: remove lazy workqueue creation
2010-10-15 13:06 ` Evgeniy Polyakov
@ 2010-10-15 13:38 ` Tejun Heo
2010-10-15 13:49 ` Evgeniy Polyakov
0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2010-10-15 13:38 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: netdev@vger.kernel.org, Frederic Weisbecker, David S. Miller
Hello,
On 10/15/2010 03:06 PM, Evgeniy Polyakov wrote:
> On Fri, Oct 15, 2010 at 11:55:22AM +0200, Tejun Heo (tj@kernel.org) wrote:
>> Commit 1a5645bc (connector: create connector workqueue only while
>> needed once) implements lazy workqueue creation for connector
>> workqueue. With cmwq now in place, lazy workqueue creation doesn't
>> make much sense while adding a lot of complexity. Remove it and
>> allocate an ordered workqueue during initialization.
>>
>> This also removes a call to flush_scheduled_work() which is deprecated
>> and scheduled to be removed.
>
> Looks good, thank you.
> Ack.
Are you gonna route this patch? If not, I can route it through
workqueue tree.
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connector: remove lazy workqueue creation
2010-10-15 13:38 ` Tejun Heo
@ 2010-10-15 13:49 ` Evgeniy Polyakov
2010-10-15 13:50 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Evgeniy Polyakov @ 2010-10-15 13:49 UTC (permalink / raw)
To: Tejun Heo; +Cc: netdev@vger.kernel.org, Frederic Weisbecker, David S. Miller
On Fri, Oct 15, 2010 at 03:38:13PM +0200, Tejun Heo (tj@kernel.org) wrote:
> Are you gonna route this patch? If not, I can route it through
> workqueue tree.
If it does not depend on other work, I believe Dave will pull into
network tree.
--
Evgeniy Polyakov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connector: remove lazy workqueue creation
2010-10-15 13:49 ` Evgeniy Polyakov
@ 2010-10-15 13:50 ` Tejun Heo
0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2010-10-15 13:50 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: netdev@vger.kernel.org, Frederic Weisbecker, David S. Miller
On 10/15/2010 03:49 PM, Evgeniy Polyakov wrote:
> On Fri, Oct 15, 2010 at 03:38:13PM +0200, Tejun Heo (tj@kernel.org) wrote:
>> Are you gonna route this patch? If not, I can route it through
>> workqueue tree.
>
> If it does not depend on other work, I believe Dave will pull into
> network tree.
Nope, there's no dependency, so network tree then.
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connector: remove lazy workqueue creation
2010-10-15 9:55 [PATCH] connector: remove lazy workqueue creation Tejun Heo
2010-10-15 13:06 ` Evgeniy Polyakov
@ 2010-10-24 21:21 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2010-10-24 21:21 UTC (permalink / raw)
To: tj; +Cc: zbr, netdev, fweisbec
From: Tejun Heo <tj@kernel.org>
Date: Fri, 15 Oct 2010 11:55:22 +0200
> Commit 1a5645bc (connector: create connector workqueue only while
> needed once) implements lazy workqueue creation for connector
> workqueue. With cmwq now in place, lazy workqueue creation doesn't
> make much sense while adding a lot of complexity. Remove it and
> allocate an ordered workqueue during initialization.
>
> This also removes a call to flush_scheduled_work() which is deprecated
> and scheduled to be removed.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
Applied, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-24 21:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-15 9:55 [PATCH] connector: remove lazy workqueue creation Tejun Heo
2010-10-15 13:06 ` Evgeniy Polyakov
2010-10-15 13:38 ` Tejun Heo
2010-10-15 13:49 ` Evgeniy Polyakov
2010-10-15 13:50 ` Tejun Heo
2010-10-24 21:21 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).