* [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks
@ 2012-09-13 14:36 Karsten Keil
2012-09-13 18:59 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Karsten Keil @ 2012-09-13 14:36 UTC (permalink / raw)
To: davem; +Cc: netdev, stable
It is a bad idea to hold a spinlock and call flush_work_sync.
Move the workqueue cleanup outside the spinlock and use cancel_work_sync,
on closing the channel this seems to be the more correct function.
Remove the never used and constant return value of mISDN_freebchannel.
Signed-off-by: Karsten Keil <keil@b1-systems.de>
Cc: <stable@kernel.org>
---
drivers/isdn/hardware/mISDN/avmfritz.c | 3 ++-
drivers/isdn/hardware/mISDN/mISDNipac.c | 3 ++-
drivers/isdn/hardware/mISDN/mISDNisar.c | 3 ++-
drivers/isdn/hardware/mISDN/netjet.c | 3 ++-
drivers/isdn/hardware/mISDN/w6692.c | 3 ++-
drivers/isdn/mISDN/hwchannel.c | 9 ++++-----
include/linux/mISDNhw.h | 2 +-
7 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/isdn/hardware/mISDN/avmfritz.c b/drivers/isdn/hardware/mISDN/avmfritz.c
index fa6ca47..dceaec8 100644
--- a/drivers/isdn/hardware/mISDN/avmfritz.c
+++ b/drivers/isdn/hardware/mISDN/avmfritz.c
@@ -857,8 +857,9 @@ avm_bctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
test_and_clear_bit(FLG_OPEN, &bch->Flags);
+ cancel_work_sync(&bch->workq);
spin_lock_irqsave(&fc->lock, flags);
- mISDN_freebchannel(bch);
+ mISDN_clear_bchannel(bch);
modehdlc(bch, ISDN_P_NONE);
spin_unlock_irqrestore(&fc->lock, flags);
ch->protocol = ISDN_P_NONE;
diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c b/drivers/isdn/hardware/mISDN/mISDNipac.c
index 752e082..ccd7d85 100644
--- a/drivers/isdn/hardware/mISDN/mISDNipac.c
+++ b/drivers/isdn/hardware/mISDN/mISDNipac.c
@@ -1406,8 +1406,9 @@ hscx_bctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
test_and_clear_bit(FLG_OPEN, &bch->Flags);
+ cancel_work_sync(&bch->workq);
spin_lock_irqsave(hx->ip->hwlock, flags);
- mISDN_freebchannel(bch);
+ mISDN_clear_bchannel(bch);
hscx_mode(hx, ISDN_P_NONE);
spin_unlock_irqrestore(hx->ip->hwlock, flags);
ch->protocol = ISDN_P_NONE;
diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c
index be5973d..182ecf0 100644
--- a/drivers/isdn/hardware/mISDN/mISDNisar.c
+++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
@@ -1588,8 +1588,9 @@ isar_bctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
test_and_clear_bit(FLG_OPEN, &bch->Flags);
+ cancel_work_sync(&bch->workq);
spin_lock_irqsave(ich->is->hwlock, flags);
- mISDN_freebchannel(bch);
+ mISDN_clear_bchannel(bch);
modeisar(ich, ISDN_P_NONE);
spin_unlock_irqrestore(ich->is->hwlock, flags);
ch->protocol = ISDN_P_NONE;
diff --git a/drivers/isdn/hardware/mISDN/netjet.c b/drivers/isdn/hardware/mISDN/netjet.c
index c3e3e76..9bcade5 100644
--- a/drivers/isdn/hardware/mISDN/netjet.c
+++ b/drivers/isdn/hardware/mISDN/netjet.c
@@ -812,8 +812,9 @@ nj_bctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
test_and_clear_bit(FLG_OPEN, &bch->Flags);
+ cancel_work_sync(&bch->workq);
spin_lock_irqsave(&card->lock, flags);
- mISDN_freebchannel(bch);
+ mISDN_clear_bchannel(bch);
mode_tiger(bc, ISDN_P_NONE);
spin_unlock_irqrestore(&card->lock, flags);
ch->protocol = ISDN_P_NONE;
diff --git a/drivers/isdn/hardware/mISDN/w6692.c b/drivers/isdn/hardware/mISDN/w6692.c
index 26a86b8..335fe64 100644
--- a/drivers/isdn/hardware/mISDN/w6692.c
+++ b/drivers/isdn/hardware/mISDN/w6692.c
@@ -1054,8 +1054,9 @@ w6692_bctrl(struct mISDNchannel *ch, u32 cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
test_and_clear_bit(FLG_OPEN, &bch->Flags);
+ cancel_work_sync(&bch->workq);
spin_lock_irqsave(&card->lock, flags);
- mISDN_freebchannel(bch);
+ mISDN_clear_bchannel(bch);
w6692_mode(bc, ISDN_P_NONE);
spin_unlock_irqrestore(&card->lock, flags);
ch->protocol = ISDN_P_NONE;
diff --git a/drivers/isdn/mISDN/hwchannel.c b/drivers/isdn/mISDN/hwchannel.c
index ef34fd4..2602be2 100644
--- a/drivers/isdn/mISDN/hwchannel.c
+++ b/drivers/isdn/mISDN/hwchannel.c
@@ -148,17 +148,16 @@ mISDN_clear_bchannel(struct bchannel *ch)
ch->next_minlen = ch->init_minlen;
ch->maxlen = ch->init_maxlen;
ch->next_maxlen = ch->init_maxlen;
+ skb_queue_purge(&ch->rqueue);
+ ch->rcount = 0;
}
EXPORT_SYMBOL(mISDN_clear_bchannel);
-int
+void
mISDN_freebchannel(struct bchannel *ch)
{
+ cancel_work_sync(&ch->workq);
mISDN_clear_bchannel(ch);
- skb_queue_purge(&ch->rqueue);
- ch->rcount = 0;
- flush_work_sync(&ch->workq);
- return 0;
}
EXPORT_SYMBOL(mISDN_freebchannel);
diff --git a/include/linux/mISDNhw.h b/include/linux/mISDNhw.h
index d0752ec..9d96d5d 100644
--- a/include/linux/mISDNhw.h
+++ b/include/linux/mISDNhw.h
@@ -183,7 +183,7 @@ extern int mISDN_initbchannel(struct bchannel *, unsigned short,
unsigned short);
extern int mISDN_freedchannel(struct dchannel *);
extern void mISDN_clear_bchannel(struct bchannel *);
-extern int mISDN_freebchannel(struct bchannel *);
+extern void mISDN_freebchannel(struct bchannel *);
extern int mISDN_ctrl_bchannel(struct bchannel *, struct mISDN_ctrl_req *);
extern void queue_ch_frame(struct mISDNchannel *, u_int,
int, struct sk_buff *);
--
1.7.7
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks
2012-09-13 14:36 [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks Karsten Keil
@ 2012-09-13 18:59 ` David Miller
2012-09-13 19:06 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2012-09-13 18:59 UTC (permalink / raw)
To: keil; +Cc: netdev, stable
From: Karsten Keil <keil@b1-systems.de>
Date: Thu, 13 Sep 2012 16:36:20 +0200
> It is a bad idea to hold a spinlock and call flush_work_sync.
> Move the workqueue cleanup outside the spinlock and use cancel_work_sync,
> on closing the channel this seems to be the more correct function.
> Remove the never used and constant return value of mISDN_freebchannel.
>
> Signed-off-by: Karsten Keil <keil@b1-systems.de>
> Cc: <stable@kernel.org>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks
2012-09-13 18:59 ` David Miller
@ 2012-09-13 19:06 ` David Miller
2012-09-13 19:50 ` Karsten Keil
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2012-09-13 19:06 UTC (permalink / raw)
To: keil; +Cc: netdev, stable
From: David Miller <davem@davemloft.net>
Date: Thu, 13 Sep 2012 14:59:37 -0400 (EDT)
> From: Karsten Keil <keil@b1-systems.de>
> Date: Thu, 13 Sep 2012 16:36:20 +0200
>
>> It is a bad idea to hold a spinlock and call flush_work_sync.
>> Move the workqueue cleanup outside the spinlock and use cancel_work_sync,
>> on closing the channel this seems to be the more correct function.
>> Remove the never used and constant return value of mISDN_freebchannel.
>>
>> Signed-off-by: Karsten Keil <keil@b1-systems.de>
>> Cc: <stable@kernel.org>
>
> Applied, thanks.
BTW, about -stable:
1) Even if it were appropriate to submit this directly to -stable,
stable@kernel.org is not the correct email address and you must
have seen the bounce produced by trying to send email there.
Rather, stable@vger.kernel.org is the correct address.
2) I queue up and submit networking bug fixes for -stable myself so
you should not submit them directly but rather make a request that
I add your patch to my networking -stable queue.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks
2012-09-13 19:06 ` David Miller
@ 2012-09-13 19:50 ` Karsten Keil
0 siblings, 0 replies; 4+ messages in thread
From: Karsten Keil @ 2012-09-13 19:50 UTC (permalink / raw)
To: David Miller; +Cc: keil, netdev
Am 13.09.2012 21:06, schrieb David Miller:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 13 Sep 2012 14:59:37 -0400 (EDT)
>
>> From: Karsten Keil <keil@b1-systems.de>
>> Date: Thu, 13 Sep 2012 16:36:20 +0200
>>
>>> It is a bad idea to hold a spinlock and call flush_work_sync.
>>> Move the workqueue cleanup outside the spinlock and use cancel_work_sync,
>>> on closing the channel this seems to be the more correct function.
>>> Remove the never used and constant return value of mISDN_freebchannel.
>>>
>>> Signed-off-by: Karsten Keil <keil@b1-systems.de>
>>> Cc: <stable@kernel.org>
>>
>> Applied, thanks.
>
> BTW, about -stable:
>
> 1) Even if it were appropriate to submit this directly to -stable,
> stable@kernel.org is not the correct email address and you must
> have seen the bounce produced by trying to send email there.
>
> Rather, stable@vger.kernel.org is the correct address.
>
> 2) I queue up and submit networking bug fixes for -stable myself so
> you should not submit them directly but rather make a request that
> I add your patch to my networking -stable queue.
>
OK, then I will put something like, "Should be considered for stable
too" in the comment and do not add the CC: stable@vger.kernel.org
for the next time.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-13 19:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-13 14:36 [PATCH] mISDN: Fix wrong usage of flush_work_sync while holding locks Karsten Keil
2012-09-13 18:59 ` David Miller
2012-09-13 19:06 ` David Miller
2012-09-13 19:50 ` Karsten Keil
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).