The allocation was moved to probe function in 9842c38e9176. And we can
sleep there. So allocate the 4*8192 bytes as GFP_KERNEL to mitigate
the allocation failure.
Signed-off-by: Jiri Slaby <[email protected]>
Tested-by: Gerald Pfeifer <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/nozomi.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index fd0a9852..acaecc1 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -1431,8 +1431,8 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
}
for (i = PORT_MDM; i < MAX_PORT; i++) {
- if (kfifo_alloc(&dc->port[i].fifo_ul,
- FIFO_BUFFER_SIZE_UL, GFP_ATOMIC)) {
+ if (kfifo_alloc(&dc->port[i].fifo_ul, FIFO_BUFFER_SIZE_UL,
+ GFP_KERNEL)) {
dev_err(&pdev->dev,
"Could not allocate kfifo buffer\n");
ret = -ENOMEM;
--
1.7.4.2
moxa_write can be called from atomic context with irqs disabled (from
ppp_async_push). Don't enable interrupts by spin_unlock_bh as this
might cause deadlocks in the ppp layer.
Instead, use irqsave/irqrestore spin_lock functions.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/moxa.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index a290e9e..6255561 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -1202,14 +1202,15 @@ static int moxa_write(struct tty_struct *tty,
const unsigned char *buf, int count)
{
struct moxa_port *ch = tty->driver_data;
+ unsigned long flags;
int len;
if (ch == NULL)
return 0;
- spin_lock_bh(&moxa_lock);
+ spin_lock_irqsave(&moxa_lock, flags);
len = MoxaPortWriteData(tty, buf, count);
- spin_unlock_bh(&moxa_lock);
+ spin_unlock_irqrestore(&moxa_lock, flags);
set_bit(LOWWAIT, &ch->statusflags);
return len;
--
1.7.4.2
tty_sem used to protect tty open count. This was removed in 33dd474a
but the lock remained in place.
So remove it completely as it protects nothing now.
Also this solves Mac's problem with inatomic operation called from
atomic context (ppp):
BUG: scheduling while atomic: firefox-bin/1992/0x10000800
Modules linked in: ...
Pid: 1992, comm: firefox-bin Not tainted 2.6.38 #1
Call Trace:
...
[] ? mutex_lock+0xe/0x21
[] ? ntty_write+0x5d/0x192 [nozomi]
[] ? __mod_timer.clone.30+0xbe/0xcc
[] ? check_preempt_curr+0x60/0x6d
[] ? __nf_ct_refresh_acct+0x75/0xbe
[] ? ppp_async_push+0xa9/0x3bd [ppp_async]
[] ? ppp_async_send+0x34/0x40 [ppp_async]
[] ? ppp_push+0x6c/0x4f9 [ppp_generic]
...
Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: Mac <[email protected]>
Tested-by: Gerald Pfeifer <[email protected]>
Cc: Jack Stone <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/nozomi.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index c34d622..b1aecc7 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -364,8 +364,6 @@ struct port {
u8 toggle_ul;
u16 token_dl;
- /* mutex to ensure one access patch to this port */
- struct mutex tty_sem;
wait_queue_head_t tty_wait;
struct async_icount tty_icount;
@@ -1474,7 +1472,6 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
struct device *tty_dev;
struct port *port = &dc->port[i];
port->dc = dc;
- mutex_init(&port->tty_sem);
tty_port_init(&port->port);
port->port.ops = &noz_tty_port_ops;
tty_dev = tty_register_device(ntty_driver, dc->index_start + i,
@@ -1688,8 +1685,6 @@ static int ntty_write(struct tty_struct *tty, const unsigned char *buffer,
if (!dc || !port)
return -ENODEV;
- mutex_lock(&port->tty_sem);
-
rval = kfifo_in(&port->fifo_ul, (unsigned char *)buffer, count);
/* notify card */
@@ -1714,7 +1709,6 @@ static int ntty_write(struct tty_struct *tty, const unsigned char *buffer,
spin_unlock_irqrestore(&dc->spin_mutex, flags);
exit:
- mutex_unlock(&port->tty_sem);
return rval;
}
@@ -1733,11 +1727,9 @@ static int ntty_write_room(struct tty_struct *tty)
int room = 4096;
const struct nozomi *dc = get_dc_by_tty(tty);
- if (dc) {
- mutex_lock(&port->tty_sem);
+ if (dc)
room = kfifo_avail(&port->fifo_ul);
- mutex_unlock(&port->tty_sem);
- }
+
return room;
}
--
1.7.4.2
tty_write_lock and tty_write_unlock contain imbalanced locking. But
this is intentional, so mark them appropriately by
__acquires/__releases.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/tty_io.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0bcf388..7721d6d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -964,12 +964,14 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
}
void tty_write_unlock(struct tty_struct *tty)
+ __releases(&tty->atomic_write_lock)
{
mutex_unlock(&tty->atomic_write_lock);
wake_up_interruptible_poll(&tty->write_wait, POLLOUT);
}
int tty_write_lock(struct tty_struct *tty, int ndelay)
+ __acquires(&tty->atomic_write_lock)
{
if (!mutex_trylock(&tty->atomic_write_lock)) {
if (ndelay)
--
1.7.4.2
Before 33dd474a, these were some kind of protection against race with
HUP. They were protected with port->tty_sem at the same time.
By that commit, the counting was switched to tty_port's one, but the
locking remained the old one. So the count was not protected by
any lock anymore.
The driver should not test whether it raced with HUP or not anyways.
With the new refcounted tty model, it just should proceed as nothing
happened because all needed info is still there. In respect to this,
let's drop the useless and unprotected tests (tty_port->count is
protected by tty_port->lock).
Signed-off-by: Jiri Slaby <[email protected]>
Tested-by: Gerald Pfeifer <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/nozomi.c | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index acaecc1..c34d622 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -1690,11 +1690,6 @@ static int ntty_write(struct tty_struct *tty, const unsigned char *buffer,
mutex_lock(&port->tty_sem);
- if (unlikely(!port->port.count)) {
- DBG1(" ");
- goto exit;
- }
-
rval = kfifo_in(&port->fifo_ul, (unsigned char *)buffer, count);
/* notify card */
@@ -1740,8 +1735,7 @@ static int ntty_write_room(struct tty_struct *tty)
if (dc) {
mutex_lock(&port->tty_sem);
- if (port->port.count)
- room = kfifo_avail(&port->fifo_ul);
+ room = kfifo_avail(&port->fifo_ul);
mutex_unlock(&port->tty_sem);
}
return room;
@@ -1889,11 +1883,6 @@ static s32 ntty_chars_in_buffer(struct tty_struct *tty)
goto exit_in_buffer;
}
- if (unlikely(!port->port.count)) {
- dev_err(&dc->pdev->dev, "No tty open?\n");
- goto exit_in_buffer;
- }
-
rval = kfifo_len(&port->fifo_ul);
exit_in_buffer:
--
1.7.4.2
msleep* is guaranteed to return with TASK_RUNNING task state. And
since there is no other set_task_state in the paths of
uart_wait_until_sent, we need not to set_task_state to TASK_RUNNING.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/serial/serial_core.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 848fd13..cda1089 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1427,7 +1427,6 @@ static void __uart_wait_until_sent(struct uart_port *port, int timeout)
if (time_after(jiffies, expire))
break;
}
- set_current_state(TASK_RUNNING); /* might not be needed */
}
static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
--
1.7.4.2
tty->index (named here as line) is set up in initialize_tty_struct.
The value is checked in get_tty_driver for the found driver as:
if (device < base || device >= base + p->num)
continue;
*index = device - base;
So index/line can never be more than driver->num. Hence remove this
test from uart_open.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/tty/serial/serial_core.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d4bd465..848fd13 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1543,15 +1543,6 @@ static int uart_open(struct tty_struct *tty, struct file *filp)
pr_debug("uart_open(%d) called\n", line);
/*
- * tty->driver->num won't change, so we won't fail here with
- * tty->driver_data set to something non-NULL (and therefore
- * we won't get caught by uart_close()).
- */
- retval = -ENODEV;
- if (line >= tty->driver->num)
- goto fail;
-
- /*
* We take the semaphore inside uart_get to guarantee that we won't
* be re-entered while allocating the state structure, or while we
* request any IRQs that the driver may need. This also has the nice
--
1.7.4.2
On 20/04/2011 09:43, Jiri Slaby wrote:
> tty_sem used to protect tty open count. This was removed in 33dd474a
> but the lock remained in place.
>
> So remove it completely as it protects nothing now.
>
> Also this solves Mac's problem with inatomic operation called from
> atomic context (ppp):
> BUG: scheduling while atomic: firefox-bin/1992/0x10000800
> Modules linked in: ...
> Pid: 1992, comm: firefox-bin Not tainted 2.6.38 #1
> Call Trace:
> ...
> [] ? mutex_lock+0xe/0x21
> [] ? ntty_write+0x5d/0x192 [nozomi]
> [] ? __mod_timer.clone.30+0xbe/0xcc
> [] ? check_preempt_curr+0x60/0x6d
> [] ? __nf_ct_refresh_acct+0x75/0xbe
> [] ? ppp_async_push+0xa9/0x3bd [ppp_async]
> [] ? ppp_async_send+0x34/0x40 [ppp_async]
> [] ? ppp_push+0x6c/0x4f9 [ppp_generic]
> ...
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Reported-by: Mac <[email protected]>
> Tested-by: Gerald Pfeifer <[email protected]>
> Cc: Jack Stone <[email protected]>
Reviewed-by: Jack Stone <[email protected]>
> Cc: Alan Cox <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
Thanks for sorting this Jiri.
Jack