LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
@ 2008-02-05 19:02 Rick Warner
  2008-02-05 20:18 ` Paul Fulghum
  0 siblings, 1 reply; 8+ messages in thread
From: Rick Warner @ 2008-02-05 19:02 UTC (permalink / raw)
  To: linux-kernel

Hello all,

My company uses a proprietary hardware monitoring solution that utilizes 
communication over the serial port.  A RS232-RS485 converter is plugged into 
the serial port of the master of a cluster, and cat5 cables are daisy chained 
between the cards to handle out of band hardware monitoring.  The cards 
themselves speak RS485.  We have been using the same software to read the 
data from the serial port since the 2.4 kernel days.   As of 2.6.23, the 
software no longer works.

I narrowed down the problem doing a binary search on git snapshots between .22 
and .23, and found the breakage between git6 and git7.  Further isolating it 
found the patch mentioned in the subject to be the cause.  I reversed the 
patch in the .23 source and it now works properly.

Should the code be reverted back as I did, or is there something I should 
change in our userspace code that reads from the serial port to correct it 
instead?

Thanks,
Rick

-- 
Richard Warner
Lead Systems Integrator
Microway, Inc
(508)732-5517

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 19:02 serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch Rick Warner
@ 2008-02-05 20:18 ` Paul Fulghum
  2008-02-05 20:53   ` Paul Fulghum
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Fulghum @ 2008-02-05 20:18 UTC (permalink / raw)
  To: Rick Warner; +Cc: linux-kernel

Rick Warner wrote:
> I narrowed down the problem doing a binary search on git snapshots between .22 
> and .23, and found the breakage between git6 and git7.  Further isolating it 
> found the patch mentioned in the subject to be the cause.  I reversed the 
> patch in the .23 source and it now works properly.
> 
> Should the code be reverted back as I did, or is there something I should 
> change in our userspace code that reads from the serial port to correct it 
> instead?

Instead of reverting the patch can you try modifying
this part of the patch:

+	if (wait_event_interruptible_timeout(tty->write_wait,
+			!tty->driver->chars_in_buffer(tty), timeout))
+		return;

by changing it to:

+	if (wait_event_interruptible_timeout(tty->write_wait,
+			!tty->driver->chars_in_buffer(tty), timeout) < 0)
+		return;

It looks like the patch changed the behavior of
tty_wait_until_sent by not calling the driver
specific wait_until_sent if a timeout occurs.

-- 
Paul Fulghum
Microgate Systems, Ltd.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 20:18 ` Paul Fulghum
@ 2008-02-05 20:53   ` Paul Fulghum
  2008-02-05 21:32     ` Rick Warner
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Fulghum @ 2008-02-05 20:53 UTC (permalink / raw)
  To: Rick Warner; +Cc: linux-kernel

Paul Fulghum wrote:
> Instead of reverting the patch can you try modifying
> this part of the patch:
> 
> +	if (wait_event_interruptible_timeout(tty->write_wait,
> +			!tty->driver->chars_in_buffer(tty), timeout))
> +		return;
> 
> by changing it to:
> 
> +	if (wait_event_interruptible_timeout(tty->write_wait,
> +			!tty->driver->chars_in_buffer(tty), timeout) < 0)
> +		return;
> 
> It looks like the patch changed the behavior of
> tty_wait_until_sent by not calling the driver
> specific wait_until_sent if a timeout occurs.

I mispoke, the patch changed the behavior by not
calling the driver specific wait_until_sent if
the condition is true.

Original behavior:
call driver->wait_until_sent() on
timeout or true condition
(skip for signal)

Patch behavior:
call driver->wait_until_sent() only
on timeout (rc == 0)
(skip for signal or true)

By modifying the patch as described above,
the original behavior is restored.

-- 
Paul Fulghum
Microgate Systems, Ltd.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 20:53   ` Paul Fulghum
@ 2008-02-05 21:32     ` Rick Warner
  2008-02-05 21:37       ` Paul Fulghum
  0 siblings, 1 reply; 8+ messages in thread
From: Rick Warner @ 2008-02-05 21:32 UTC (permalink / raw)
  To: Paul Fulghum, linux-kernel

This modification solved my problem.  Will this change go into mainline, or 
will we need to maintain our own branch of the kernel to keep this working?

Thanks,
Rick

On Tuesday 05 February 2008, Paul Fulghum wrote:
> Paul Fulghum wrote:
> > Instead of reverting the patch can you try modifying
> > this part of the patch:
> >
> > +	if (wait_event_interruptible_timeout(tty->write_wait,
> > +			!tty->driver->chars_in_buffer(tty), timeout))
> > +		return;
> >
> > by changing it to:
> >
> > +	if (wait_event_interruptible_timeout(tty->write_wait,
> > +			!tty->driver->chars_in_buffer(tty), timeout) < 0)
> > +		return;
> >
> > It looks like the patch changed the behavior of
> > tty_wait_until_sent by not calling the driver
> > specific wait_until_sent if a timeout occurs.
>
> I mispoke, the patch changed the behavior by not
> calling the driver specific wait_until_sent if
> the condition is true.
>
> Original behavior:
> call driver->wait_until_sent() on
> timeout or true condition
> (skip for signal)
>
> Patch behavior:
> call driver->wait_until_sent() only
> on timeout (rc == 0)
> (skip for signal or true)
>
> By modifying the patch as described above,
> the original behavior is restored.



-- 
Richard Warner
Lead Systems Integrator
Microway, Inc
(508)732-5517

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 21:32     ` Rick Warner
@ 2008-02-05 21:37       ` Paul Fulghum
  2008-02-05 21:59         ` Jiri Slaby
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Fulghum @ 2008-02-05 21:37 UTC (permalink / raw)
  To: Rick Warner; +Cc: linux-kernel, Jiri Slaby

Rick Warner wrote:
> This modification solved my problem.  Will this change go into mainline, or 
> will we need to maintain our own branch of the kernel to keep this working?

It should be accepted into mainline as it restores
the original behavior. I'll put together a patch
submission tomorrow unless Jiri beats me to it.

Thanks,
Paul

-- 
Paul Fulghum
Microgate Systems, Ltd.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 21:37       ` Paul Fulghum
@ 2008-02-05 21:59         ` Jiri Slaby
  2008-02-05 22:27           ` Paul Fulghum
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Slaby @ 2008-02-05 21:59 UTC (permalink / raw)
  To: Paul Fulghum; +Cc: Rick Warner, linux-kernel, Jiri Slaby

On 02/05/2008 10:37 PM, Paul Fulghum wrote:
> Rick Warner wrote:
>> This modification solved my problem.  Will this change go into 
>> mainline, or will we need to maintain our own branch of the kernel to 
>> keep this working?
> 
> It should be accepted into mainline as it restores
> the original behavior. I'll put together a patch
> submission tomorrow unless Jiri beats me to it.

It should be fixed already as of
git-describe db99247a
v2.6.24-rc6-95-gdb99247

So since 2.6.24-rc7.

Maybe we should consider the fix for 2.6.23 too.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 21:59         ` Jiri Slaby
@ 2008-02-05 22:27           ` Paul Fulghum
  2008-02-05 22:33             ` Jiri Slaby
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Fulghum @ 2008-02-05 22:27 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Rick Warner, linux-kernel

Jiri Slaby wrote:
> It should be fixed already as of
> git-describe db99247a
> v2.6.24-rc6-95-gdb99247
> 
> So since 2.6.24-rc7.
> 
> Maybe we should consider the fix for 2.6.23 too.

Whoops, I missed that.
Problem solved :-)

Thanks,
Paul

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch
  2008-02-05 22:27           ` Paul Fulghum
@ 2008-02-05 22:33             ` Jiri Slaby
  0 siblings, 0 replies; 8+ messages in thread
From: Jiri Slaby @ 2008-02-05 22:33 UTC (permalink / raw)
  To: Paul Fulghum; +Cc: Rick Warner, linux-kernel

On 02/05/2008 11:27 PM, Paul Fulghum wrote:
> Jiri Slaby wrote:
>> It should be fixed already as of
>> git-describe db99247a
>> v2.6.24-rc6-95-gdb99247
>>
>> So since 2.6.24-rc7.
>>
>> Maybe we should consider the fix for 2.6.23 too.
> 
> Whoops, I missed that.
> Problem solved :-)

:) Thank you both for heads up.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-02-05 22:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-05 19:02 serial port regression caused by "Char: tty_ioctl, use wait_event_interruptible_timeout" patch Rick Warner
2008-02-05 20:18 ` Paul Fulghum
2008-02-05 20:53   ` Paul Fulghum
2008-02-05 21:32     ` Rick Warner
2008-02-05 21:37       ` Paul Fulghum
2008-02-05 21:59         ` Jiri Slaby
2008-02-05 22:27           ` Paul Fulghum
2008-02-05 22:33             ` Jiri Slaby

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).