* [PATCH, RFC 17/30] atm: nicstar: remove interruptible_sleep_on_timeout
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 18/30] atm: firestream: fix interruptible_sleep_on race Arnd Bergmann
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, David S. Miller, Chas Williams, linux-atm-general,
netdev
We are trying to finally kill off interruptible_sleep_on_timeout.
the two uses in the nicstar driver can be trivially replaced
with wait_event_interruptible_lock_irq_timeout, which prevents the
wake-up race and is able to check the buffer state with scq->lock
held.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Chas Williams <chas@cmf.nrl.navy.mil>
Cc: linux-atm-general@lists.sourceforge.net
Cc: netdev@vger.kernel.org
---
drivers/atm/nicstar.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
index 5aca5f4..3c079eb 100644
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -1739,10 +1739,9 @@ static int push_scqe(ns_dev * card, vc_map * vc, scq_info * scq, ns_scqe * tbd,
}
scq->full = 1;
- spin_unlock_irqrestore(&scq->lock, flags);
- interruptible_sleep_on_timeout(&scq->scqfull_waitq,
- SCQFULL_TIMEOUT);
- spin_lock_irqsave(&scq->lock, flags);
+ wait_event_interruptible_lock_irq_timeout(scq->scqfull_waitq,
+ scq->tail != scq->next, scq->lock,
+ SCQFULL_TIMEOUT);
if (scq->full) {
spin_unlock_irqrestore(&scq->lock, flags);
@@ -1789,10 +1788,10 @@ static int push_scqe(ns_dev * card, vc_map * vc, scq_info * scq, ns_scqe * tbd,
scq->full = 1;
if (has_run++)
break;
- spin_unlock_irqrestore(&scq->lock, flags);
- interruptible_sleep_on_timeout(&scq->scqfull_waitq,
+ wait_event_interruptible_lock_irq_timeout(scq->scqfull_waitq,
+ scq->tail != scq->next,
+ scq->lock,
SCQFULL_TIMEOUT);
- spin_lock_irqsave(&scq->lock, flags);
}
if (!scq->full) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH, RFC 18/30] atm: firestream: fix interruptible_sleep_on race
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 17/30] atm: nicstar: remove interruptible_sleep_on_timeout Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 19/30] isdn: pcbit: " Arnd Bergmann
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Chas Williams, linux-atm-general, netdev
interruptible_sleep_on is racy and going away. This replaces the one use
in the firestream driver with the appropriate wait_event_interruptible
variant.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Chas Williams <chas@cmf.nrl.navy.mil>
Cc: linux-atm-general@lists.sourceforge.net
Cc: netdev@vger.kernel.org
---
drivers/atm/firestream.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index b41c948..f43e1c1 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -736,8 +736,8 @@ static void process_txdone_queue (struct fs_dev *dev, struct queue *q)
skb = td->skb;
if (skb == FS_VCC (ATM_SKB(skb)->vcc)->last_skb) {
- wake_up_interruptible (& FS_VCC (ATM_SKB(skb)->vcc)->close_wait);
FS_VCC (ATM_SKB(skb)->vcc)->last_skb = NULL;
+ wake_up_interruptible (& FS_VCC (ATM_SKB(skb)->vcc)->close_wait);
}
td->dev->ntxpckts--;
@@ -1123,7 +1123,7 @@ static void fs_close(struct atm_vcc *atm_vcc)
this sleep_on, we'll lose any reference to these packets. Memory leak!
On the other hand, it's awfully convenient that we can abort a "close" that
is taking too long. Maybe just use non-interruptible sleep on? -- REW */
- interruptible_sleep_on (& vcc->close_wait);
+ wait_event_interruptible(vcc->close_wait, !vcc->last_skb);
}
txtp = &atm_vcc->qos.txtp;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH, RFC 19/30] isdn: pcbit: fix interruptible_sleep_on race
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 17/30] atm: nicstar: remove interruptible_sleep_on_timeout Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 18/30] atm: firestream: fix interruptible_sleep_on race Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 20/30] isdn: hisax/elsa: fix sleep_on race in elsa FSM Arnd Bergmann
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Karsten Keil, netdev
interruptible_sleep_on is racy and going away. In case of pcbit,
the driver would run into a timeout if the card is initialized
before we start waiting for it. This uses wait_event to fix the
race. In order to do this, the state machine handling for the
timeout case has to get trivially reorganized so we actually know
whether the timeout has occorred or not.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
---
drivers/isdn/pcbit/drv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/pcbit/drv.c b/drivers/isdn/pcbit/drv.c
index 1eaf622..f02cc50 100644
--- a/drivers/isdn/pcbit/drv.c
+++ b/drivers/isdn/pcbit/drv.c
@@ -796,6 +796,7 @@ static void set_running_timeout(unsigned long ptr)
#endif
dev = (struct pcbit_dev *) ptr;
+ dev->l2_state = L2_DOWN;
wake_up_interruptible(&dev->set_running_wq);
}
@@ -818,7 +819,8 @@ static int set_protocol_running(struct pcbit_dev *dev)
add_timer(&dev->set_running_timer);
- interruptible_sleep_on(&dev->set_running_wq);
+ wait_event(dev->set_running_wq, dev->l2_state == L2_RUNNING ||
+ dev->l2_state == L2_DOWN);
del_timer(&dev->set_running_timer);
@@ -842,8 +844,6 @@ static int set_protocol_running(struct pcbit_dev *dev)
printk(KERN_DEBUG "pcbit: initialization failed\n");
printk(KERN_DEBUG "pcbit: firmware not loaded\n");
- dev->l2_state = L2_DOWN;
-
#ifdef DEBUG
printk(KERN_DEBUG "Bank3 = %02x\n",
readb(dev->sh_mem + BANK3));
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH, RFC 20/30] isdn: hisax/elsa: fix sleep_on race in elsa FSM
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
` (2 preceding siblings ...)
2014-01-02 12:07 ` [PATCH, RFC 19/30] isdn: pcbit: " Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race Arnd Bergmann
2014-01-02 12:07 ` [PATCH, RFC 22/30] isdn: fix multiple sleep_on races Arnd Bergmann
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Karsten Keil, netdev
The state machine code in the elsa driver uses interruptible_sleep_on
to wait for state changes, which is racy. A closer look at the possible
states reveals that it is always used to wait for getting back into
ARCOFI_NOP, so we can use wait_event_interruptible instead.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
---
drivers/isdn/hisax/elsa.c | 9 ++++++---
drivers/isdn/hisax/elsa_ser.c | 3 ++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/isdn/hisax/elsa.c b/drivers/isdn/hisax/elsa.c
index 2be1c8a..d8ef64d 100644
--- a/drivers/isdn/hisax/elsa.c
+++ b/drivers/isdn/hisax/elsa.c
@@ -509,7 +509,8 @@ static void
set_arcofi(struct IsdnCardState *cs, int bc) {
cs->dc.isac.arcofi_bc = bc;
arcofi_fsm(cs, ARCOFI_START, &ARCOFI_COP_5);
- interruptible_sleep_on(&cs->dc.isac.arcofi_wait);
+ wait_event_interruptible(cs->dc.isac.arcofi_wait,
+ cs->dc.isac.arcofi_state == ARCOFI_NOP);
}
static int
@@ -528,7 +529,8 @@ check_arcofi(struct IsdnCardState *cs)
}
cs->dc.isac.arcofi_bc = 0;
arcofi_fsm(cs, ARCOFI_START, &ARCOFI_VERSION);
- interruptible_sleep_on(&cs->dc.isac.arcofi_wait);
+ wait_event_interruptible(cs->dc.isac.arcofi_wait,
+ cs->dc.isac.arcofi_state == ARCOFI_NOP);
if (!test_and_clear_bit(FLG_ARCOFI_ERROR, &cs->HW_Flags)) {
debugl1(cs, "Arcofi response received %d bytes", cs->dc.isac.mon_rxp);
p = cs->dc.isac.mon_rx;
@@ -595,7 +597,8 @@ check_arcofi(struct IsdnCardState *cs)
Elsa_Types[cs->subtyp],
cs->hw.elsa.base + 8);
arcofi_fsm(cs, ARCOFI_START, &ARCOFI_XOP_0);
- interruptible_sleep_on(&cs->dc.isac.arcofi_wait);
+ wait_event_interruptible(cs->dc.isac.arcofi_wait,
+ cs->dc.isac.arcofi_state == ARCOFI_NOP);
return (1);
}
return (0);
diff --git a/drivers/isdn/hisax/elsa_ser.c b/drivers/isdn/hisax/elsa_ser.c
index 3f84dd8..a2a358c 100644
--- a/drivers/isdn/hisax/elsa_ser.c
+++ b/drivers/isdn/hisax/elsa_ser.c
@@ -573,7 +573,8 @@ modem_l2l1(struct PStack *st, int pr, void *arg)
test_and_clear_bit(BC_FLG_ACTIV, &bcs->Flag);
bcs->cs->dc.isac.arcofi_bc = st->l1.bc;
arcofi_fsm(bcs->cs, ARCOFI_START, &ARCOFI_XOP_0);
- interruptible_sleep_on(&bcs->cs->dc.isac.arcofi_wait);
+ wait_event_interruptible(bcs->cs->dc.isac.arcofi_wait,
+ bcs->cs->dc.isac.arcofi_state == ARCOFI_NOP);
bcs->cs->hw.elsa.MFlag = 1;
} else {
printk(KERN_WARNING "ElsaSer: unknown pr %x\n", pr);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
` (3 preceding siblings ...)
2014-01-02 12:07 ` [PATCH, RFC 20/30] isdn: hisax/elsa: fix sleep_on race in elsa FSM Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
2014-01-02 15:01 ` Sergei Shtylyov
2014-01-02 12:07 ` [PATCH, RFC 22/30] isdn: fix multiple sleep_on races Arnd Bergmann
5 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Karsten Keil, netdev
These two drivers use identical code for their procfs status
file handling, which contains a small race against status
data becoming available while reading the file.
This uses wait_event_interruptible instead to fix this
particular race and eventually get rid of all sleep_on
instances. There seems to be another race involving
multiple concurrent readers of the same procfs file, which
I don't try to fix here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
---
drivers/isdn/divert/divert_procfs.c | 7 ++++---
drivers/isdn/hysdn/hysdn_proclog.c | 7 ++++---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
index fb4f1ba..1c5dc34 100644
--- a/drivers/isdn/divert/divert_procfs.c
+++ b/drivers/isdn/divert/divert_procfs.c
@@ -86,12 +86,13 @@ isdn_divert_read(struct file *file, char __user *buf, size_t count, loff_t *off)
struct divert_info *inf;
int len;
- if (!*((struct divert_info **) file->private_data)) {
+ if (!(inf = *((struct divert_info **) file->private_data))) {
if (file->f_flags & O_NONBLOCK)
return -EAGAIN;
- interruptible_sleep_on(&(rd_queue));
+ wait_event_interruptible(rd_queue, (inf =
+ *((struct divert_info **) file->private_data)));
}
- if (!(inf = *((struct divert_info **) file->private_data)))
+ if (!inf)
return (0);
inf->usage_cnt--; /* new usage count */
diff --git a/drivers/isdn/hysdn/hysdn_proclog.c b/drivers/isdn/hysdn/hysdn_proclog.c
index b61e8d5..7b5fd8f 100644
--- a/drivers/isdn/hysdn/hysdn_proclog.c
+++ b/drivers/isdn/hysdn/hysdn_proclog.c
@@ -175,14 +175,15 @@ hysdn_log_read(struct file *file, char __user *buf, size_t count, loff_t *off)
int len;
hysdn_card *card = PDE_DATA(file_inode(file));
- if (!*((struct log_data **) file->private_data)) {
+ if (!(inf = *((struct log_data **) file->private_data))) {
struct procdata *pd = card->proclog;
if (file->f_flags & O_NONBLOCK)
return (-EAGAIN);
- interruptible_sleep_on(&(pd->rd_queue));
+ wait_event_interruptible(pd->rd_queue, (inf =
+ *((struct log_data **) file->private_data)));
}
- if (!(inf = *((struct log_data **) file->private_data)))
+ if (!inf)
return (0);
inf->usage_cnt--; /* new usage count */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race
2014-01-02 12:07 ` [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race Arnd Bergmann
@ 2014-01-02 15:01 ` Sergei Shtylyov
2014-01-02 16:48 ` Arnd Bergmann
0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2014-01-02 15:01 UTC (permalink / raw)
To: Arnd Bergmann, linux-kernel; +Cc: Karsten Keil, netdev
Hello.
On 02-01-2014 16:07, Arnd Bergmann wrote:
> These two drivers use identical code for their procfs status
> file handling, which contains a small race against status
> data becoming available while reading the file.
> This uses wait_event_interruptible instead to fix this
> particular race and eventually get rid of all sleep_on
> instances. There seems to be another race involving
> multiple concurrent readers of the same procfs file, which
> I don't try to fix here.
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Karsten Keil <isdn@linux-pingi.de>
> Cc: netdev@vger.kernel.org
> ---
> drivers/isdn/divert/divert_procfs.c | 7 ++++---
> drivers/isdn/hysdn/hysdn_proclog.c | 7 ++++---
> 2 files changed, 8 insertions(+), 6 deletions(-)
> diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
> index fb4f1ba..1c5dc34 100644
> --- a/drivers/isdn/divert/divert_procfs.c
> +++ b/drivers/isdn/divert/divert_procfs.c
> @@ -86,12 +86,13 @@ isdn_divert_read(struct file *file, char __user *buf, size_t count, loff_t *off)
> struct divert_info *inf;
> int len;
>
> - if (!*((struct divert_info **) file->private_data)) {
> + if (!(inf = *((struct divert_info **) file->private_data))) {
checkpatch.pl shouldn't approve assignment inside *if*. Though you're
moving it from the existing code, it wouldn't hurt to fix it.
> if (file->f_flags & O_NONBLOCK)
> return -EAGAIN;
> - interruptible_sleep_on(&(rd_queue));
> + wait_event_interruptible(rd_queue, (inf =
> + *((struct divert_info **) file->private_data)));
Parens around assignment are hardly useful.
> }
> - if (!(inf = *((struct divert_info **) file->private_data)))
> + if (!inf)
> return (0);
>
> inf->usage_cnt--; /* new usage count */
> diff --git a/drivers/isdn/hysdn/hysdn_proclog.c b/drivers/isdn/hysdn/hysdn_proclog.c
> index b61e8d5..7b5fd8f 100644
> --- a/drivers/isdn/hysdn/hysdn_proclog.c
> +++ b/drivers/isdn/hysdn/hysdn_proclog.c
> @@ -175,14 +175,15 @@ hysdn_log_read(struct file *file, char __user *buf, size_t count, loff_t *off)
> int len;
> hysdn_card *card = PDE_DATA(file_inode(file));
>
> - if (!*((struct log_data **) file->private_data)) {
> + if (!(inf = *((struct log_data **) file->private_data))) {
Here too checkpatch.pk should complain...
> struct procdata *pd = card->proclog;
> if (file->f_flags & O_NONBLOCK)
> return (-EAGAIN);
>
> - interruptible_sleep_on(&(pd->rd_queue));
> + wait_event_interruptible(pd->rd_queue, (inf =
> + *((struct log_data **) file->private_data)));
And parens hardly needed.
WBR, Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race
2014-01-02 15:01 ` Sergei Shtylyov
@ 2014-01-02 16:48 ` Arnd Bergmann
2014-01-02 23:00 ` Sergei Shtylyov
0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 16:48 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-kernel, Karsten Keil, netdev
On Thursday 02 January 2014 19:01:15 Sergei Shtylyov wrote:
> > diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
> > index fb4f1ba..1c5dc34 100644
> > --- a/drivers/isdn/divert/divert_procfs.c
> > +++ b/drivers/isdn/divert/divert_procfs.c
> > @@ -86,12 +86,13 @@ isdn_divert_read(struct file *file, char __user *buf, size_t count, loff_t *off)
> > struct divert_info *inf;
> > int len;
> >
> > - if (!*((struct divert_info **) file->private_data)) {
> > + if (!(inf = *((struct divert_info **) file->private_data))) {
>
> checkpatch.pl shouldn't approve assignment inside *if*. Though you're
> moving it from the existing code, it wouldn't hurt to fix it.
I tried to touch as little as possible, and while I wouldn't use that
style myself, it is applied consistently in this driver, including the
wait_event line I'm adding, where I feel it actually makes sense.
> > if (file->f_flags & O_NONBLOCK)
> > return -EAGAIN;
> > - interruptible_sleep_on(&(rd_queue));
> > + wait_event_interruptible(rd_queue, (inf =
> > + *((struct divert_info **) file->private_data)));
>
> Parens around assignment are hardly useful.
We get a gcc warning without them:
drivers/isdn/divert/divert_procfs.c:95:14: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
*((struct divert_info **) file->private_data));
I can still change the first one (in both files) if you think it's important,
but I'd rather not spend too much energy at coding style changes.
Thanks for taking a look.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race
2014-01-02 16:48 ` Arnd Bergmann
@ 2014-01-02 23:00 ` Sergei Shtylyov
0 siblings, 0 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2014-01-02 23:00 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, Karsten Keil, netdev
Hello.
On 01/02/2014 07:48 PM, Arnd Bergmann wrote:
>>> diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
>>> index fb4f1ba..1c5dc34 100644
>>> --- a/drivers/isdn/divert/divert_procfs.c
>>> +++ b/drivers/isdn/divert/divert_procfs.c
>>> @@ -86,12 +86,13 @@ isdn_divert_read(struct file *file, char __user *buf, size_t count, loff_t *off)
>>> struct divert_info *inf;
>>> int len;
>>>
>>> - if (!*((struct divert_info **) file->private_data)) {
>>> + if (!(inf = *((struct divert_info **) file->private_data))) {
>> checkpatch.pl shouldn't approve assignment inside *if*. Though you're
>> moving it from the existing code, it wouldn't hurt to fix it.
> I tried to touch as little as possible, and while I wouldn't use that
> style myself, it is applied consistently in this driver, including the
> wait_event line I'm adding, where I feel it actually makes sense.
I wasn't feeling sure about that one. It seems to me now that it doesn't
make much sense -- we could do the assignment beforehand.
>>> if (file->f_flags & O_NONBLOCK)
>>> return -EAGAIN;
>>> - interruptible_sleep_on(&(rd_queue));
>>> + wait_event_interruptible(rd_queue, (inf =
>>> + *((struct divert_info **) file->private_data)));
>> Parens around assignment are hardly useful.
> We get a gcc warning without them:
> drivers/isdn/divert/divert_procfs.c:95:14: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
> *((struct divert_info **) file->private_data));
Ah...
> I can still change the first one (in both files) if you think it's important,
I always prefer checkpatch.pl-clean patches, though some people do argue
when they are just moving the badly styled code around.
> but I'd rather not spend too much energy at coding style changes.
Let's wait for the maintainer's opinion on this.
> Arnd
WBR, Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH, RFC 22/30] isdn: fix multiple sleep_on races
2014-01-02 12:07 [PATCH, RFC 00/30] sleep_on removal Arnd Bergmann
` (4 preceding siblings ...)
2014-01-02 12:07 ` [PATCH, RFC 21/30] isdn: divert, hysdn: fix interruptible_sleep_on race Arnd Bergmann
@ 2014-01-02 12:07 ` Arnd Bergmann
5 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2014-01-02 12:07 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Karsten Keil, netdev
The isdn core code uses a couple of wait queues with
interruptible_sleep_on, which is racy and about to get
removed from the kernel. Fortunately, we know for each case
what we are waiting for, so they can all be converted to
the better wait_event_interruptible interface.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
---
drivers/isdn/i4l/isdn_common.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
index 9bb12ba..130f216 100644
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -777,7 +777,8 @@ isdn_readbchan(int di, int channel, u_char *buf, u_char *fp, int len, wait_queue
return 0;
if (skb_queue_empty(&dev->drv[di]->rpqueue[channel])) {
if (sleep)
- interruptible_sleep_on(sleep);
+ wait_event_interruptible(*sleep,
+ !skb_queue_empty(&dev->drv[di]->rpqueue[channel]));
else
return 0;
}
@@ -1072,7 +1073,8 @@ isdn_read(struct file *file, char __user *buf, size_t count, loff_t *off)
retval = -EAGAIN;
goto out;
}
- interruptible_sleep_on(&(dev->info_waitq));
+ wait_event_interruptible(dev->info_waitq,
+ file->private_data);
}
p = isdn_statstr();
file->private_data = NULL;
@@ -1128,7 +1130,8 @@ isdn_read(struct file *file, char __user *buf, size_t count, loff_t *off)
retval = -EAGAIN;
goto out;
}
- interruptible_sleep_on(&(dev->drv[drvidx]->st_waitq));
+ wait_event_interruptible(dev->drv[drvidx]->st_waitq,
+ dev->drv[drvidx]->stavail);
}
if (dev->drv[drvidx]->interface->readstat) {
if (count > dev->drv[drvidx]->stavail)
@@ -1188,8 +1191,8 @@ isdn_write(struct file *file, const char __user *buf, size_t count, loff_t *off)
goto out;
}
chidx = isdn_minor2chan(minor);
- while ((retval = isdn_writebuf_stub(drvidx, chidx, buf, count)) == 0)
- interruptible_sleep_on(&dev->drv[drvidx]->snd_waitq[chidx]);
+ wait_event_interruptible(dev->drv[drvidx]->snd_waitq[chidx],
+ (retval = isdn_writebuf_stub(drvidx, chidx, buf, count)));
goto out;
}
if (minor <= ISDN_MINOR_CTRLMAX) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread