Hi,
> > __cancel_delayed_work() is being used in some paths where we cannot
> > sleep waiting for the delayed work to finish. However, that function
> > might return while the timer is running and the work will be queued
> > again. Replace the calls with safer cancel_delayed_work() version
> > which spins until the timer handler finishes on other CPUs and
> > cancels the delayed work.
> >
> > Signed-off-by: Ulisses Furquim <ulisses@xxxxxxxxxxxxxx>
> > ---
> > include/net/bluetooth/l2cap.h | 4 ++--
> > net/bluetooth/l2cap_core.c | 6 +++---
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index e7a8cc7..42fdbb8 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -614,7 +614,7 @@ static inline void l2cap_set_timer(struct l2cap_chan *chan,
> > {
> > BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout);
> >
> > - if (!__cancel_delayed_work(work))
> > + if (!cancel_delayed_work(work))
> > l2cap_chan_hold(chan);
> > schedule_delayed_work(work, timeout);
> > }
> > @@ -624,7 +624,7 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
> > {
> > bool ret;
> >
> > - ret = __cancel_delayed_work(work);
> > + ret = cancel_delayed_work(work);
> > if (ret)
> > l2cap_chan_put(chan);
> >
>
>
> I have some questions about delayed_work usage:
>
> When setting timer with l2cap_set_timer() we hold_chan if work may be
> running. So if previous work is cancelled OK we do not hold chan.
>
> Didn't we miss hold_chan here?
>
> Then in l2cap_clear_chan we put_chan if work cancelled OK, otherwise
> put_chan is done in delayed_work so we always put_chan.
What about following change:
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a357336..d61e158 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -628,17 +628,6 @@ static inline void l2cap_chan_unlock(struct
l2cap_chan *chan)
mutex_unlock(&chan->lock);
}
-static inline void l2cap_set_timer(struct l2cap_chan *chan,
- struct delayed_work *work, long
timeout)
-{
- BT_DBG("chan %p state %s timeout %ld", chan,
- state_to_string(chan->state),
timeout);
-
- if (!cancel_delayed_work(work))
- l2cap_chan_hold(chan);
- schedule_delayed_work(work, timeout);
-}
-
static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
struct delayed_work *work)
{
@@ -651,6 +640,18 @@ static inline bool l2cap_clear_timer(struct
l2cap_chan *chan,
return ret;
}
+static inline void l2cap_set_timer(struct l2cap_chan *chan,
+ struct delayed_work *work, long
timeout)
+{
+ BT_DBG("chan %p state %s timeout %ld", chan,
+ state_to_string(chan->state),
timeout);
+
+ l2cap_clear_timer(chan, work);
+
+ l2cap_chan_hold(chan);
+ schedule_delayed_work(work, timeout);
+}
+
#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
#define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \
Best regards
Andrei Emeltchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
