Signed-off-by: Al Viro <[email protected]>
---
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 5e7cd45..4cec1a8 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -135,8 +135,7 @@ static int cn_call_callback(struct cn_ms
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(__cbq, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
- if (likely(!test_bit(WORK_STRUCT_PENDING,
- &__cbq->work.work.management) &&
+ if (likely(!work_pending(&__cbq->work.work) &&
__cbq->data.ddata == NULL)) {
__cbq->data.callback_priv = msg;
On Sun, 17 Dec 2006, Al Viro wrote:
> - if (likely(!test_bit(WORK_STRUCT_PENDING,
> - &__cbq->work.work.management) &&
> + if (likely(!work_pending(&__cbq->work.work) &&
That should properly be
if (likely(!delayed_work_pending(&__cbq->work) && ...
and why the heck was it doing that open-coded int he first place?
HOWEVER, looking even more, why is that thing a "delayed work" at all? All
the queuing seems to happen with a timeout of zero..
So I _think_ that the proper patch is actually the following, but somebody
who knows and uses the connector thing should double-check. Please?
Linus
---
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index b418b16..296f510 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -34,7 +34,7 @@
void cn_queue_wrapper(struct work_struct *work)
{
struct cn_callback_entry *cbq =
- container_of(work, struct cn_callback_entry, work.work);
+ container_of(work, struct cn_callback_entry, work);
struct cn_callback_data *d = &cbq->data;
d->callback(d->callback_priv);
@@ -59,13 +59,12 @@ static struct cn_callback_entry *cn_queue_alloc_callback_entry(char *name, struc
memcpy(&cbq->id.id, id, sizeof(struct cb_id));
cbq->data.callback = callback;
- INIT_DELAYED_WORK(&cbq->work, &cn_queue_wrapper);
+ INIT_WORK(&cbq->work, &cn_queue_wrapper);
return cbq;
}
static void cn_queue_free_callback(struct cn_callback_entry *cbq)
{
- cancel_delayed_work(&cbq->work);
flush_workqueue(cbq->pdev->cn_queue);
kfree(cbq);
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 5e7cd45..416de8c 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -135,17 +135,16 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(__cbq, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
- if (likely(!test_bit(WORK_STRUCT_PENDING,
- &__cbq->work.work.management) &&
+ if (likely(!work_pending(&__cbq->work) &&
__cbq->data.ddata == NULL)) {
__cbq->data.callback_priv = msg;
__cbq->data.ddata = data;
__cbq->data.destruct_data = destruct_data;
- if (queue_delayed_work(
+ if (queue_work(
dev->cbdev->cn_queue,
- &__cbq->work, 0))
+ &__cbq->work))
err = 0;
} else {
struct cn_callback_data *d;
@@ -159,12 +158,12 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
d->destruct_data = destruct_data;
d->free = __cbq;
- INIT_DELAYED_WORK(&__cbq->work,
+ INIT_WORK(&__cbq->work,
&cn_queue_wrapper);
- if (queue_delayed_work(
+ if (queue_work(
dev->cbdev->cn_queue,
- &__cbq->work, 0))
+ &__cbq->work))
err = 0;
else {
kfree(__cbq);
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 3ea1cd5..10eb56b 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -133,7 +133,7 @@ struct cn_callback_data {
struct cn_callback_entry {
struct list_head callback_entry;
struct cn_callback *cb;
- struct delayed_work work;
+ struct work_struct work;
struct cn_queue_dev *pdev;
struct cn_callback_id id;
On Sun, Dec 17, 2006 at 09:24:30AM -0800, Linus Torvalds ([email protected]) wrote:
>
>
> On Sun, 17 Dec 2006, Al Viro wrote:
> > - if (likely(!test_bit(WORK_STRUCT_PENDING,
> > - &__cbq->work.work.management) &&
> > + if (likely(!work_pending(&__cbq->work.work) &&
>
> That should properly be
>
> if (likely(!delayed_work_pending(&__cbq->work) && ...
>
> and why the heck was it doing that open-coded int he first place?
>
> HOWEVER, looking even more, why is that thing a "delayed work" at all? All
> the queuing seems to happen with a timeout of zero..
>
> So I _think_ that the proper patch is actually the following, but somebody
> who knows and uses the connector thing should double-check. Please?
Delayed work was used to play with different timeouts and thus allow to
smooth performance peaks, but then I dropped that idea, so timeout is always
zero.
I posted similar patch today to netdev@, which directly used
work_pending instead of delayed_work_pending(), but if you will figure
this out itself, I'm ok with proposed patch.
> Linus
--
Evgeniy Polyakov
On Sun, 17 Dec 2006, Evgeniy Polyakov wrote:
>
> Delayed work was used to play with different timeouts and thus allow to
> smooth performance peaks, but then I dropped that idea, so timeout is always
> zero.
Ok, thanks for the explanation.
> I posted similar patch today to netdev@, which directly used
> work_pending instead of delayed_work_pending(), but if you will figure
> this out itself, I'm ok with proposed patch.
If I'm going to get the proper patch from the proper network trees, I'll
just drop my patch. Whether you replace "delayed_work" with "work_struct"
or not is not something I really care about - if you think you may want to
play with the timeout idea in the future, please feel free to continue
using delayed_work.
But if you do use delayed work, please use the "delayed_work_pending(&x)"
function, rather than doing "work_pending(&x->work)" and knowing about the
internals of how the delayed-work structure looks.
So with that out of the way, I'll just expect that I'll get whatever you
decide on through Davem's git tree, once his drunken holiday revelry is
over ;)
Linus
On Sun, Dec 17, 2006 at 10:08:49AM -0800, Linus Torvalds ([email protected]) wrote:
> So with that out of the way, I'll just expect that I'll get whatever you
> decide on through Davem's git tree, once his drunken holiday revelry is
> over ;)
This is important process - never interrupt it for things like patches
from external developers :)
I will push it through his tree.
> Linus
--
Evgeniy Polyakov