2009-03-14 21:36:44

by Jan Kasprzak

[permalink] [raw]
Subject: MOXA I/O errors on 2.6.28+

Hello,

I have several servers with MOXA C320 serial boards. Starting with 2.6.28,
some ports on MOXA boards sometimes get to a state where open("/dev/ttyMX<n>")
returns -EIO. I use the servers with MOXA boards as serial console hubs,
and I think the EIO state is caused by the remote getty or the whole remote
server being restarted.

After getting EIO, this particular port always returns EIO, until
the server is rebooted. The moxa module usage count is non-zero (even when
nobody has the board open), so reloading just the moxa.ko module is not
possible.

2.6.27 is OK, 2.6.28 and 2.6.29-rc8 is faulty. I can git bisect
if you want (I have a spare server which I can reboot at will).

My MOXA boards are

01:08.0 Serial controller: Moxa Technologies Co Ltd Intellio C320 Turbo PCI (rev 02)

What can I do to have this problem fixed? Thanks,

-Yenya

--
| Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/ Journal: http://www.fi.muni.cz/~kas/blog/ |
>> If you find yourself arguing with Alan Cox, you’re _probably_ wrong. <<
>> --James Morris in "How and Why You Should Become a Kernel Hacker" <<


2009-03-14 21:43:00

by Alan

[permalink] [raw]
Subject: Re: MOXA I/O errors on 2.6.28+

On Sat, 14 Mar 2009 22:36:30 +0100
Jan Kasprzak <[email protected]> wrote:

> Hello,
>
> I have several servers with MOXA C320 serial boards. Starting with 2.6.28,
> some ports on MOXA boards sometimes get to a state where open("/dev/ttyMX<n>")
> returns -EIO. I use the servers with MOXA boards as serial console hubs,
> and I think the EIO state is caused by the remote getty or the whole remote
> server being restarted.

Sounds like a refcount bug.

> 2.6.27 is OK, 2.6.28 and 2.6.29-rc8 is faulty. I can git bisect
> if you want (I have a spare server which I can reboot at will).

Might be useful but let me take a look first, chances are you've provided
enough information
>
> My MOXA boards are
>
> 01:08.0 Serial controller: Moxa Technologies Co Ltd Intellio C320 Turbo PCI (rev 02)
>
> What can I do to have this problem fixed? Thanks,

Step 1: Report it giving accurate detailed info... which you've done
nicely.

Alan

2009-03-14 22:41:59

by Jan Kasprzak

[permalink] [raw]
Subject: Re: MOXA I/O errors on 2.6.28+

Alan Cox wrote:
: On Sat, 14 Mar 2009 22:36:30 +0100
: Jan Kasprzak <[email protected]> wrote:
:
: > I have several servers with MOXA C320 serial boards. Starting with 2.6.28,
: > some ports on MOXA boards sometimes get to a state where open("/dev/ttyMX<n>")
: > returns -EIO. I use the servers with MOXA boards as serial console hubs,
: > and I think the EIO state is caused by the remote getty or the whole remote
: > server being restarted.
:
: Sounds like a refcount bug.
:
: > 2.6.27 is OK, 2.6.28 and 2.6.29-rc8 is faulty. I can git bisect
: > if you want (I have a spare server which I can reboot at will).
:
: Might be useful but let me take a look first, chances are you've provided
: enough information

It is this one:

d450b5a0196b6442cf3f29fc611d9c8daa56b559 tty: kref usage for isicom and moxa.

Reverting this commit on top of 2.6.29-rc8 (and resolving a single conflict
the revert introduces in drivers/char/moxa.c) makes the problem disappear.

-Yenya

--
| Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/ Journal: http://www.fi.muni.cz/~kas/blog/ |
>> If you find yourself arguing with Alan Cox, you’re _probably_ wrong. <<
>> --James Morris in "How and Why You Should Become a Kernel Hacker" <<

2009-03-15 09:31:29

by Jiri Slaby

[permalink] [raw]
Subject: Re: MOXA I/O errors on 2.6.28+

On 14.3.2009 23:41, Jan Kasprzak wrote:
> Alan Cox wrote:
> : Sounds like a refcount bug.
...
> d450b5a0196b6442cf3f29fc611d9c8daa56b559 tty: kref usage for isicom and moxa.

Thanks a heap. moxa_poll_port is a good candidate for fixing. A patch
will follow, could you try it?

2009-03-15 09:37:19

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 1/1] Char: moxa, fix refcounting in moxa_poll_port

There is missing tty_kref_put on some paths in moxa_poll_port,
although the reference is always taken. Fix it.

Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: Jan 'Yenya' Kasprzak <[email protected]>
Cc: Alan Cox <[email protected]>
---
drivers/char/moxa.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index 8b0da97..4a4cab7 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -1486,11 +1486,11 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
}

if (!handle) /* nothing else to do */
- return 0;
+ goto put;

intr = readw(ip); /* port irq status */
if (intr == 0)
- return 0;
+ goto put;

writew(0, ip); /* ACK port */
ofsAddr = p->tableAddr;
@@ -1499,16 +1499,17 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
ofsAddr + HostStat);

if (!inited)
- return 0;
+ goto put;

if (tty && (intr & IntrBreak) && !I_IGNBRK(tty)) { /* BREAK */
tty_insert_flip_char(tty, 0, TTY_BREAK);
tty_schedule_flip(tty);
}
- tty_kref_put(tty);

if (intr & IntrLine)
moxa_new_dcdstate(p, readb(ofsAddr + FlagStat) & DCD_state);
+put:
+ tty_kref_put(tty);

return 0;
}
--
1.6.2

2009-03-15 09:38:27

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 1/1] Char: moxa, fix refcounting in moxa_poll_port

Repost, sorry, scripts don't understand Reported-by.

--

There is missing tty_kref_put on some paths in moxa_poll_port,
although the reference is always taken. Fix it.

Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: Jan 'Yenya' Kasprzak <[email protected]>
Cc: Alan Cox <[email protected]>
---
drivers/char/moxa.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index 8b0da97..4a4cab7 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -1486,11 +1486,11 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
}

if (!handle) /* nothing else to do */
- return 0;
+ goto put;

intr = readw(ip); /* port irq status */
if (intr == 0)
- return 0;
+ goto put;

writew(0, ip); /* ACK port */
ofsAddr = p->tableAddr;
@@ -1499,16 +1499,17 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
ofsAddr + HostStat);

if (!inited)
- return 0;
+ goto put;

if (tty && (intr & IntrBreak) && !I_IGNBRK(tty)) { /* BREAK */
tty_insert_flip_char(tty, 0, TTY_BREAK);
tty_schedule_flip(tty);
}
- tty_kref_put(tty);

if (intr & IntrLine)
moxa_new_dcdstate(p, readb(ofsAddr + FlagStat) & DCD_state);
+put:
+ tty_kref_put(tty);

return 0;
}
--
1.6.2

2009-03-15 11:52:23

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 1/1] Char: moxa, fix refcounting in moxa_poll_port

Restoring CCs.

On 15.3.2009 11:55, Alan Cox wrote:
> On Sun, 15 Mar 2009 10:36:08 +0100
> Jiri Slaby<[email protected]> wrote:
>
>> There is missing tty_kref_put on some paths in moxa_poll_port,
>> although the reference is always taken. Fix it.
>>
>> Signed-off-by: Jiri Slaby<[email protected]>
>> Reported-by: Jan 'Yenya' Kasprzak<[email protected]>
>> Cc: Alan Cox<[email protected]>
>
> Acked-by: Alan Cox<[email protected]>

BTW the same sha, the change in isicom_shutdown_port doesn't seem to be
quite right too. Well, the change itself is, but not in the code
context. The tty is used even after tty_port_tty_set(NULL). This is
wrong, isn't it?

2009-03-15 15:26:21

by Jan Kasprzak

[permalink] [raw]
Subject: Re: [PATCH 1/1] Char: moxa, fix refcounting in moxa_poll_port

Jiri Slaby wrote:
: Repost, sorry, scripts don't understand Reported-by.

Looks OK, and works for me (against 2.6.29-rc8).
Thanks for the fast response :-).

-Yenya

: Signed-off-by: Jiri Slaby <[email protected]>
: Reported-by: Jan 'Yenya' Kasprzak <[email protected]>
: Cc: Alan Cox <[email protected]>

Acked-by: Jan "Yenya" Kasprzak <[email protected]>

: ---
: drivers/char/moxa.c | 9 +++++----
: 1 files changed, 5 insertions(+), 4 deletions(-)
:
: diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
: index 8b0da97..4a4cab7 100644
: --- a/drivers/char/moxa.c
: +++ b/drivers/char/moxa.c
: @@ -1486,11 +1486,11 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
: }
:
: if (!handle) /* nothing else to do */
: - return 0;
: + goto put;
:
: intr = readw(ip); /* port irq status */
: if (intr == 0)
: - return 0;
: + goto put;
:
: writew(0, ip); /* ACK port */
: ofsAddr = p->tableAddr;
: @@ -1499,16 +1499,17 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
: ofsAddr + HostStat);
:
: if (!inited)
: - return 0;
: + goto put;
:
: if (tty && (intr & IntrBreak) && !I_IGNBRK(tty)) { /* BREAK */
: tty_insert_flip_char(tty, 0, TTY_BREAK);
: tty_schedule_flip(tty);
: }
: - tty_kref_put(tty);
:
: if (intr & IntrLine)
: moxa_new_dcdstate(p, readb(ofsAddr + FlagStat) & DCD_state);
: +put:
: + tty_kref_put(tty);
:
: return 0;
: }
: --
: 1.6.2
:
: --
: To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
: the body of a message to [email protected]
: More majordomo info at http://vger.kernel.org/majordomo-info.html
: Please read the FAQ at http://www.tux.org/lkml/

--
| Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/ Journal: http://www.fi.muni.cz/~kas/blog/ |
>> If you find yourself arguing with Alan Cox, you’re _probably_ wrong. <<
>> --James Morris in "How and Why You Should Become a Kernel Hacker" <<