Re: n_tty_write() going into schedule but NOT coming out

[Ilya Zykov]

Please, try this patch maybe it can help localize your problem.

 drivers/tty/n_tty.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 05e72be..28f15d0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -230,6 +230,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
ldata->canon_head = ldata->canon_data = ldata->erasing = 0;
bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
n_tty_set_room(tty);
+   check_unthrottle(tty);
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: n_tty_write() going into schedule but NOT coming out

[Ilya Zykov]

Please, try this patch maybe it can help localize your problem.

 drivers/tty/n_tty.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 05e72be..28f15d0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -230,6 +230,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
ldata-canon_head = ldata-canon_data = ldata-erasing = 0;
bitmap_zero(ldata-read_flags, N_TTY_BUF_SIZE);
n_tty_set_room(tty);
+   check_unthrottle(tty);
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

On 20.03.2013 21:49, Peter Hurley wrote:
> The motivation for changing the workqueue api to allow parallel work
> items on SMP was to fix a class of deadlocks where forward progress
> could not be made due to subtle dependencies between work items
> (actually that potential still exists with self-modifying work-items,
> ie., work items that change their function).
> 
> The tty layer would need a detailed and thorough analysis of potential
> dependencies to avoid creating problems. The drivers that use work items
> might need examination as well.

Sorry, but I don't understand.
My knowledge is very weak.

We say about three works:
hangup_work, SAK_work, buf.work
or anything else?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix race condition if flushing tty flip buffers

[Ilya Zykov]

On 20.03.2013 21:20, Peter Hurley wrote:
> As Ilya Zykov identified in his patch 'PROBLEM: Race condition in
> tty buffer's function flush_to_ldisc()', a race condition exists
> which allows a parallel flush_to_ldisc() to flush and free the tty
> flip buffers while those buffers are in-use. For example,
> 
>   CPU 0 |  CPU 1  |  
> CPU 2
> | flush_to_ldisc()|
> |  grab spin lock |
> tty_buffer_flush()  | | 
> flush_to_ldisc()
>  wait for spin lock | |  
> wait for spin lock
> |   if (!test_and_set_bit(TTYP_FLUSHING)) |
> |while (next flip buffer) |
> | ... |
> | drop spin lock  |
>   grab spin lock| |
>if (test_bit(TTYP_FLUSHING)) | |
> set_bit(TTYP_FLUSHPENDING)  |  receive_buf()  |
> drop spin lock  | |
> | |   
> grab spin lock
> | |   
>  if (!test_and_set_bit(TTYP_FLUSHING))
> | |   
>  if (test_bit(TTYP_FLUSHPENDING))
> | |   
>  __tty_buffer_flush()
> 
> CPU 2 has just flushed and freed all tty flip buffers while CPU 1 is
> transferring data from the head flip buffer.
> 
> The original patch was rejected under the assumption that parallel
> flush_to_ldisc() was not possible. Because of necessary changes to
> the workqueue api, work items can execute in parallel on SMP.
> 
> This patch differs slightly from the original patch by testing for
> a pending flush _after_ each receive_buf(), since TTYP_FLUSHPENDING
> can only be set while the lock is dropped around receive_buf().
> 
> Reported-by: Ilya Zykov 
> Signed-off-by: Peter Hurley 
> ---
>  drivers/tty/tty_buffer.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index bb11993..9fd8acd 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -449,11 +449,6 @@ static void flush_to_ldisc(struct work_struct *work)
>   tty_buffer_free(port, head);
>   continue;
>   }
> - /* Ldisc or user is trying to flush the buffers
> -we are feeding to the ldisc, stop feeding the
> -line discipline as we want to empty the queue */
> - if (test_bit(TTYP_FLUSHPENDING, >iflags))
> - break;
>   if (!tty->receive_room)
>   break;
>   if (count > tty->receive_room)
> @@ -465,17 +460,20 @@ static void flush_to_ldisc(struct work_struct *work)
>   disc->ops->receive_buf(tty, char_buf,
>   flag_buf, count);
>   spin_lock_irqsave(>lock, flags);
> + /* Ldisc or user is trying to flush the buffers.
> +We may have a deferred request to flush the
> +input buffer, if so pull the chain under the lock
> +and empty the queue */
> + if (test_bit(TTYP_FLUSHPENDING, >iflags)) {
> + __tty_buffer_flush(port);
> + clear_bit(TTYP_FLUSHPENDING, >iflags);
> + wake_up(>read_wait);
> + break;
> + }
>   }
>   clear_bit(TTYP_FLUSHING, >iflags);
>   }
>  
> - /* We may have a deferred request to flush the input buffer,
> -if so pull the chain under the lock and empty the queue */
> -     if (test_bit(TTYP_FLUSHPENDING, >iflags)) {
> - __tty_buffer_flush(port);
> - clear_bit(TTYP_FLUSHPENDING, >iflags);
> - wake_up(>read_wait);
> - }
>   spin_unlock_irqrestore(>lock, flags);
>  
>   tty_ldisc_deref(disc);
> 

Acked-by: Ilya Zykov 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

>> I have little question about flush_to_ldisc().
>> Does can it be multithreaded?
>>
>> I think yes, because on SMP schedule_work() can work on different CPU 
>> paralleled.
> 
> Yes, the same work item can now run in parallel on SMP since Tejun Heo
> re-did the workqueue implementation on 2.6.36 [Stefan Richter, the
> firewire maintainer, recently explained this history to me].

About multi threaded delayed works:

In many cases tty layer needs single threaded delayed work for each tty 
instance.
I propose discussion about create for this purpose (tty layer)'s workqueue with 
WQ_NON_REENTRANT flag.
And use it instead common schedule_work()'s workqueue - system_wq.
I don't know how expensive(for system resource and CPU) it can be,
but for tty layer, it can be very useful.

> 
>> What do you think about this race condition?
>> https://lkml.org/lkml/2011/11/7/98
> 
> Yes, that is a possible race condition that could lead to some nasty
> results. Good find.
> 
> If you want, I could bring that patch into this patchset or you could
> re-submit that patch to Greg and I could rebase this patchset on top of
> that.

Peter,
Please, do it anywhere you consider possible,
I can't do it myself now.

Thank you,
Ilya.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

 I have little question about flush_to_ldisc().
 Does can it be multithreaded?

 I think yes, because on SMP schedule_work() can work on different CPU 
 paralleled.
 
 Yes, the same work item can now run in parallel on SMP since Tejun Heo
 re-did the workqueue implementation on 2.6.36 [Stefan Richter, the
 firewire maintainer, recently explained this history to me].

About multi threaded delayed works:

In many cases tty layer needs single threaded delayed work for each tty 
instance.
I propose discussion about create for this purpose (tty layer)'s workqueue with 
WQ_NON_REENTRANT flag.
And use it instead common schedule_work()'s workqueue - system_wq.
I don't know how expensive(for system resource and CPU) it can be,
but for tty layer, it can be very useful.

 
 What do you think about this race condition?
 https://lkml.org/lkml/2011/11/7/98
 
 Yes, that is a possible race condition that could lead to some nasty
 results. Good find.
 
 If you want, I could bring that patch into this patchset or you could
 re-submit that patch to Greg and I could rebase this patchset on top of
 that.

Peter,
Please, do it anywhere you consider possible,
I can't do it myself now.

Thank you,
Ilya.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix race condition if flushing tty flip buffers

[Ilya Zykov]

On 20.03.2013 21:20, Peter Hurley wrote:
 As Ilya Zykov identified in his patch 'PROBLEM: Race condition in
 tty buffer's function flush_to_ldisc()', a race condition exists
 which allows a parallel flush_to_ldisc() to flush and free the tty
 flip buffers while those buffers are in-use. For example,
 
   CPU 0 |  CPU 1  |  
 CPU 2
 | flush_to_ldisc()|
 |  grab spin lock |
 tty_buffer_flush()  | | 
 flush_to_ldisc()
  wait for spin lock | |  
 wait for spin lock
 |   if (!test_and_set_bit(TTYP_FLUSHING)) |
 |while (next flip buffer) |
 | ... |
 | drop spin lock  |
   grab spin lock| |
if (test_bit(TTYP_FLUSHING)) | |
 set_bit(TTYP_FLUSHPENDING)  |  receive_buf()  |
 drop spin lock  | |
 | |   
 grab spin lock
 | |   
  if (!test_and_set_bit(TTYP_FLUSHING))
 | |   
  if (test_bit(TTYP_FLUSHPENDING))
 | |   
  __tty_buffer_flush()
 
 CPU 2 has just flushed and freed all tty flip buffers while CPU 1 is
 transferring data from the head flip buffer.
 
 The original patch was rejected under the assumption that parallel
 flush_to_ldisc() was not possible. Because of necessary changes to
 the workqueue api, work items can execute in parallel on SMP.
 
 This patch differs slightly from the original patch by testing for
 a pending flush _after_ each receive_buf(), since TTYP_FLUSHPENDING
 can only be set while the lock is dropped around receive_buf().
 
 Reported-by: Ilya Zykov li...@izyk.ru
 Signed-off-by: Peter Hurley pe...@hurleysoftware.com
 ---
  drivers/tty/tty_buffer.c | 22 ++
  1 file changed, 10 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
 index bb11993..9fd8acd 100644
 --- a/drivers/tty/tty_buffer.c
 +++ b/drivers/tty/tty_buffer.c
 @@ -449,11 +449,6 @@ static void flush_to_ldisc(struct work_struct *work)
   tty_buffer_free(port, head);
   continue;
   }
 - /* Ldisc or user is trying to flush the buffers
 -we are feeding to the ldisc, stop feeding the
 -line discipline as we want to empty the queue */
 - if (test_bit(TTYP_FLUSHPENDING, port-iflags))
 - break;
   if (!tty-receive_room)
   break;
   if (count  tty-receive_room)
 @@ -465,17 +460,20 @@ static void flush_to_ldisc(struct work_struct *work)
   disc-ops-receive_buf(tty, char_buf,
   flag_buf, count);
   spin_lock_irqsave(buf-lock, flags);
 + /* Ldisc or user is trying to flush the buffers.
 +We may have a deferred request to flush the
 +input buffer, if so pull the chain under the lock
 +and empty the queue */
 + if (test_bit(TTYP_FLUSHPENDING, port-iflags)) {
 + __tty_buffer_flush(port);
 + clear_bit(TTYP_FLUSHPENDING, port-iflags);
 + wake_up(tty-read_wait);
 + break;
 + }
   }
   clear_bit(TTYP_FLUSHING, port-iflags);
   }
  
 - /* We may have a deferred request to flush the input buffer,
 -if so pull the chain under the lock and empty the queue */
 - if (test_bit(TTYP_FLUSHPENDING, port-iflags)) {
 - __tty_buffer_flush(port);
 - clear_bit(TTYP_FLUSHPENDING, port-iflags);
 - wake_up(tty-read_wait);
 - }
   spin_unlock_irqrestore(buf-lock, flags);
  
   tty_ldisc_deref(disc);
 

Acked-by: Ilya Zykov li...@izyk.ru

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

On 20.03.2013 21:49, Peter Hurley wrote:
 The motivation for changing the workqueue api to allow parallel work
 items on SMP was to fix a class of deadlocks where forward progress
 could not be made due to subtle dependencies between work items
 (actually that potential still exists with self-modifying work-items,
 ie., work items that change their function).
 
 The tty layer would need a detailed and thorough analysis of potential
 dependencies to avoid creating problems. The drivers that use work items
 might need examination as well.

Sorry, but I don't understand.
My knowledge is very weak.

We say about three works:
hangup_work, SAK_work, buf.work
or anything else?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

On 20.03.2013 0:21, Peter Hurley wrote:
> Ldisc interface functions must be called with interrupts enabled.
> Separating the ldisc calls into a helper function simplies the
> spin lock management.
> 
> Update the buffer's read index _after_ the data has been received
> by the ldisc.
> 

Hello Peter!
It looks good for me.
I think also we can remove two variables without waste:
(char_buf), (flag_buf) and use without (>lock)
(head->char_buf_ptr + head->read), (head->char_buf_ptr + head->read),
because (head->read) guarded by (TTYP_FLUSHING).


I have little question about flush_to_ldisc().
Does can it be multithreaded?
I think yes, because on SMP schedule_work() can work on different CPU 
paralleled.

What do you think about this race condition?
https://lkml.org/lkml/2011/11/7/98

Thank you.
Ilya Zykov 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 6/7] n_tty: Fix unsafe update of available buffer space

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
> receive_room is used to control the amount of data the flip
> buffer work can push to the read buffer. This update is unsafe:
> 
>   CPU 0|  CPU 1
>|
>| n_tty_read()
>|   n_tty_set_room()
>| left = 
> n_tty_receive_buf()|
>   |
>   n_tty_set_room() |
> left =  |
> tty->receive_room = left   |
>| tty->receive_room = left
> 
> receive_room is now updated with a stale calculation of the
> available buffer space, and the subsequent work loop will likely
> overwrite unread data in the input buffer.
> 

Sounds reasonable to me.
Thank you.
Ilya Zykov 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/7] n_tty: Buffer work should not reschedule itself

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
> Although the driver-side input path must update the available
> buffer space, it should not reschedule itself. If space is still
> available and the flip buffers are not empty, flush_to_ldisc()
> will loop again.
> 

Sounds reasonable to me.
Thank you.
Ilya Zykov 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/7] n_tty: Buffer work should not reschedule itself

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
 Although the driver-side input path must update the available
 buffer space, it should not reschedule itself. If space is still
 available and the flip buffers are not empty, flush_to_ldisc()
 will loop again.
 

Sounds reasonable to me.
Thank you.
Ilya Zykov li...@izyk.ru


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 6/7] n_tty: Fix unsafe update of available buffer space

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
 receive_room is used to control the amount of data the flip
 buffer work can push to the read buffer. This update is unsafe:
 
   CPU 0|  CPU 1
|
| n_tty_read()
|   n_tty_set_room()
| left = calc of space
 n_tty_receive_buf()|
   push data to buffer|
   n_tty_set_room() |
 left = calc of space |
 tty-receive_room = left   |
| tty-receive_room = left
 
 receive_room is now updated with a stale calculation of the
 available buffer space, and the subsequent work loop will likely
 overwrite unread data in the input buffer.
 

Sounds reasonable to me.
Thank you.
Ilya Zykov li...@izyk.ru



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] tty: Simplify tty buffer/ldisc interface with helper function

[Ilya Zykov]

On 20.03.2013 0:21, Peter Hurley wrote:
 Ldisc interface functions must be called with interrupts enabled.
 Separating the ldisc calls into a helper function simplies the
 spin lock management.
 
 Update the buffer's read index _after_ the data has been received
 by the ldisc.
 

Hello Peter!
It looks good for me.
I think also we can remove two variables without waste:
(char_buf), (flag_buf) and use without (buf-lock)
(head-char_buf_ptr + head-read), (head-char_buf_ptr + head-read),
because (head-read) guarded by (TTYP_FLUSHING).


I have little question about flush_to_ldisc().
Does can it be multithreaded?
I think yes, because on SMP schedule_work() can work on different CPU 
paralleled.

What do you think about this race condition?
https://lkml.org/lkml/2011/11/7/98

Thank you.
Ilya Zykov li...@izyk.ru


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

On 03.12.2012 13:54, Ilya Zykov wrote:
>   The root of problem is carelessly zeroing pointer(in function 
> __tty_buffer_flush()),
> when another thread can use it. It can be cause of "NULL pointer dereference".
>   Main idea of the patch, this is never release last (struct tty_buffer) in 
> the active buffer.
> Only flush data for ldisc(tty->buf.head->read = tty->buf.head->commit).
> At that moment driver can collect(write) data in buffer without conflict.
> It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
> Test program and bug report you can see:
> https://lkml.org/lkml/2012/11/29/368
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ilya Zykov 
> ---
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index 6c9b7cd..4f02f9c 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
>  {
>   struct tty_buffer *thead;
>  
> - while ((thead = tty->buf.head) != NULL) {
> - tty->buf.head = thead->next;
> - tty_buffer_free(tty, thead);
> + if (tty->buf.head == NULL)
> + return;
> + while ((thead = tty->buf.head->next) != NULL) {
> + tty_buffer_free(tty, tty->buf.head);
> + tty->buf.head = thead;
>   }
> - tty->buf.tail = NULL;
> + WARN_ON(tty->buf.head != tty->buf.tail);
> + tty->buf.head->read = tty->buf.head->commit;
>  }
>  
>  /**
> 

You can include this patch, in 3.2 series , for improve stability,
it would be merged in upstream 3.9-rc1.

Thank you.
Ilya.


https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/tty_buffer.c?id=64325a3be08d364a62ee8f84b2cf86934bc2544a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

On 03.12.2012 13:54, Ilya Zykov wrote:
   The root of problem is carelessly zeroing pointer(in function 
 __tty_buffer_flush()),
 when another thread can use it. It can be cause of NULL pointer dereference.
   Main idea of the patch, this is never release last (struct tty_buffer) in 
 the active buffer.
 Only flush data for ldisc(tty-buf.head-read = tty-buf.head-commit).
 At that moment driver can collect(write) data in buffer without conflict.
 It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
 Test program and bug report you can see:
 https://lkml.org/lkml/2012/11/29/368
 
 Cc: sta...@vger.kernel.org
 Signed-off-by: Ilya Zykov i...@ilyx.ru
 ---
 diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
 index 6c9b7cd..4f02f9c 100644
 --- a/drivers/tty/tty_buffer.c
 +++ b/drivers/tty/tty_buffer.c
 @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
  {
   struct tty_buffer *thead;
  
 - while ((thead = tty-buf.head) != NULL) {
 - tty-buf.head = thead-next;
 - tty_buffer_free(tty, thead);
 + if (tty-buf.head == NULL)
 + return;
 + while ((thead = tty-buf.head-next) != NULL) {
 + tty_buffer_free(tty, tty-buf.head);
 + tty-buf.head = thead;
   }
 - tty-buf.tail = NULL;
 + WARN_ON(tty-buf.head != tty-buf.tail);
 + tty-buf.head-read = tty-buf.head-commit;
  }
  
  /**
 

You can include this patch, in 3.2 series , for improve stability,
it would be merged in upstream 3.9-rc1.

Thank you.
Ilya.


https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/tty_buffer.c?id=64325a3be08d364a62ee8f84b2cf86934bc2544a

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] n_tty: Unthrottle tty when flushing read buffer

[Ilya Zykov]

On 24.01.2013 2:36, Karthik Manamcheri wrote:
> When the tty input buffer is full and thereby throttled,
> flushing/resetting the read buffer should unthrottle to allow more
> data to be received.
> 
> Signed-off-by: Karthik Manamcheri 
> ---
>  drivers/tty/n_tty.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 
> 19083efa2314660b25e0fb5bc793af6fb7e9af57..d5cea3bb01eaeec61b577de6c58a8000412c0c37
>  100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -229,6 +229,8 @@ static void reset_buffer_flags(struct tty_struct *tty)
>   ldata->canon_head = ldata->canon_data = ldata->erasing = 0;
>   bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
>   n_tty_set_room(tty);
> +
> + check_unthrottle(tty);
>  }
>  
>  /**
> 

It's revert - tty: fix "IRQ45: nobody cared".
commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

Please, read these for this subject:
https://lkml.org/lkml/2012/11/27/440
https://lkml.org/lkml/2012/11/21/568

https://lkml.org/lkml/2012/11/22/686

It's already fixed in the:
commit a1bf9584429d61b7096f93ae09325e1ba538e9e8
tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] n_tty: Unthrottle tty when flushing read buffer

[Ilya Zykov]

On 24.01.2013 2:36, Karthik Manamcheri wrote:
 When the tty input buffer is full and thereby throttled,
 flushing/resetting the read buffer should unthrottle to allow more
 data to be received.
 
 Signed-off-by: Karthik Manamcheri karthik.manamch...@ni.com
 ---
  drivers/tty/n_tty.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
 index 
 19083efa2314660b25e0fb5bc793af6fb7e9af57..d5cea3bb01eaeec61b577de6c58a8000412c0c37
  100644
 --- a/drivers/tty/n_tty.c
 +++ b/drivers/tty/n_tty.c
 @@ -229,6 +229,8 @@ static void reset_buffer_flags(struct tty_struct *tty)
   ldata-canon_head = ldata-canon_data = ldata-erasing = 0;
   bitmap_zero(ldata-read_flags, N_TTY_BUF_SIZE);
   n_tty_set_room(tty);
 +
 + check_unthrottle(tty);
  }
  
  /**
 

It's revert - tty: fix IRQ45: nobody cared.
commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

Please, read these for this subject:
https://lkml.org/lkml/2012/11/27/440
https://lkml.org/lkml/2012/11/21/568

https://lkml.org/lkml/2012/11/22/686

It's already fixed in the:
commit a1bf9584429d61b7096f93ae09325e1ba538e9e8
tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [next-20130118] Analyzing a call-trace reproducible with pm_test/freezer [ X86|RCU|TTY|EXT4FS|JBD2|PM related? ]

[Ilya Zykov]

On 20.01.2013 2:51, Sedat Dilek wrote:
> On Sat, Jan 19, 2013 at 11:43 PM, Ilya Zykov  wrote:
>> Hello!
>> I don't expert, but
>> maybe it can help.
>>
>> I test with:
>>   while echo mem > /sys/power/state; do sleep 2; done
>> in one X-terminal, in other I trying playing with keyboard.
>> (Without playing all right I use ext3.)
> 
> Can you test with the patches listed in [1] (including two TTY patches
> from you!)?
> My base is Linux-Next (next-20130118).
> FYI: My Ubuntu/precise is EXT4FS-formatted.
> 
Ok.
but not now I have 3.00 night.
I already use this in my 3.8.0-rc3-next-20130118-pm+.
 tty: Correct tty buffer flush.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [next-20130118] Analyzing a call-trace reproducible with pm_test/freezer [ X86|RCU|TTY|EXT4FS|JBD2|PM related? ]

[Ilya Zykov]

Hello!
I don't expert, but
maybe it can help.

I test with:
  while echo mem > /sys/power/state; do sleep 2; done
in one X-terminal, in other I trying playing with keyboard.
(Without playing all right I use ext3.)
And sometimes:

WARNING: at drivers/tty/tty_buffer.c:427 flush_to_ldisc+0x52/0x192()
Hardware name: P5K Premium
tty is NULL
Restarting tasks ... 
Modules linked in: fuse autofs4 af_packet ipt_REJECT nf_conntrack_ipv4 
nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT xt_tcpudp nf_conntrack_ipv6 
nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables x_tables ipv6 
binfmt_misc uinput hid_microsoft hid_generic usbkbd usbmouse usbhid hid 
asus_atk0110 iTCO_wdt iTCO_vendor_support evdev sr_mod cdrom sg ata_generic 
pata_jmicron coretemp kvm_intel kvm microcode pcspkr i2c_i801 lpc_ich mfd_core 
sky2 snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_seq snd_seq_device 
snd_pcm snd_timer snd soundcore snd_page_alloc uhci_hcd nouveau ttm 
drm_kms_helper drm hwmon i2c_algo_bit i2c_core mxm_wmi video wmi button 
intel_agp intel_gtt agpgart
Pid: 263, comm: kworker/1:1 Not tainted 3.8.0-rc3-next-20130118-pm+ #16
Call Trace:
 [] warn_slowpath_common+0x80/0x98
 [] warn_slowpath_fmt+0x41/0x43
 [] flush_to_ldisc+0x52/0x192
 [] ? console_unlock+0x2f4/0x319
 [] process_one_work+0x1e1/0x2a0
 [] worker_thread+0x154/0x24e
 [] ? manage_workers+0x26c/0x26c
 [] kthread+0xb0/0xb8
 [] ? kthread_freezable_should_stop+0x60/0x60
 [] ret_from_fork+0x7c/0xb0
 [] ? kthread_freezable_should_stop+0x60/0x60


Also maybe you will be intresting this:
https://lkml.org/lkml/2012/12/18/368


Cpu model name  : Intel(R) Core(TM)2 Duo CPU E8400  @ 3.00GHz
My .config:
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_LOCALVERSION="-pm"
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_AUDIT_LOGINUID_IMMUTABLE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TREE_RCU=y
CONFIG_RCU_STALL_COMMON=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_IKCONFIG=m
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=20
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_PROT_NUMA_PROT_NONE=y
CONFIG_NAMESPACES=y
CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
CONFIG_PID_NS=y
CONFIG_NET_NS=y
CONFIG_UIDGID_CONVERTED=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_SYSCTL=y
CONFIG_ANON_INODES=y
CONFIG_EXPERT=y
CONFIG_HAVE_UID16=y
CONFIG_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_ALL=y
CONFIG_HOTPLUG=y
CONFIG_PRINTK=y
CONFIG_BUG=y
CONFIG_ELF_CORE=y
CONFIG_PCSPKR_PLATFORM=y
CONFIG_HAVE_PCSPKR_PLATFORM=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_EPOLL=y
CONFIG_SIGNALFD=y
CONFIG_TIMERFD=y

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(buf->head->read = buf->head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov 

---
 drivers/tty/tty_buffer.c |   92 +++---
 1 files changed, 22 insertions(+), 70 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index d6969f6..61ec4dd 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = >buf;
struct tty_buffer *thead;
 
-   while ((thead = buf->head) != NULL) {
-   buf->head = thead->next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf->head == NULL))
+   return;
+   while ((thead = buf->head->next) != NULL) {
+   tty_buffer_free(port, buf->head);
+   buf->head = thead;
}
-   buf->tail = NULL;
+   WARN_ON(buf->head != buf->tail);
+   buf->head->read = buf->head->commit;
 }
 
 /**
@@ -194,19 +197,22 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
   have queued and recycle that ? */
 }
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port->buf.lock
+ *
+ * Locking: Takes port->buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_port *port, size_t size)
 {
struct tty_bufhead *buf = >buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+   spin_lock_irqsave(>lock, flags);
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -228,31 +234,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
} else
size = left;
}
-
+   spin_unlock_irqrestore(>lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @port: tty port structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port->buf.lock
- */
-int tty_buffer_request_room(struct tty_port *port, size_t size)
-{
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(>buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(>buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -271,26 +255,18 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_port *port,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = >buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(>lock, flags);
-   space = __tty_buffer_request_room(port, goal);
-   tb = buf->tail;
+   int space = tty_buffer_request_room(port, goal);
+   struct tty_buffer *tb = port->buf.tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(>lock, flags);
break;
}
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
-   spin_unlock_irqrestore(>lock, flags);
copied += space;
chars += space;
/*

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(buf-head-read = buf-head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov i...@ilyx.ru

---
 drivers/tty/tty_buffer.c |   92 +++---
 1 files changed, 22 insertions(+), 70 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index d6969f6..61ec4dd 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = port-buf;
struct tty_buffer *thead;
 
-   while ((thead = buf-head) != NULL) {
-   buf-head = thead-next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf-head == NULL))
+   return;
+   while ((thead = buf-head-next) != NULL) {
+   tty_buffer_free(port, buf-head);
+   buf-head = thead;
}
-   buf-tail = NULL;
+   WARN_ON(buf-head != buf-tail);
+   buf-head-read = buf-head-commit;
 }
 
 /**
@@ -194,19 +197,22 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
   have queued and recycle that ? */
 }
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port-buf.lock
+ *
+ * Locking: Takes port-buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_port *port, size_t size)
 {
struct tty_bufhead *buf = port-buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+   spin_lock_irqsave(buf-lock, flags);
/* OPTIMISATION: We could keep a per tty zero sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -228,31 +234,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
} else
size = left;
}
-
+   spin_unlock_irqrestore(buf-lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @port: tty port structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port-buf.lock
- */
-int tty_buffer_request_room(struct tty_port *port, size_t size)
-{
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(port-buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(port-buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -271,26 +255,18 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_port *port,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = port-buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(buf-lock, flags);
-   space = __tty_buffer_request_room(port, goal);
-   tb = buf-tail;
+   int space = tty_buffer_request_room(port, goal);
+   struct tty_buffer *tb = port-buf.tail;
/* If there is no space then tb may be NULL */
if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(buf-lock, flags);
break;
}
memcpy(tb-char_buf_ptr + tb-used, chars, space);
memset(tb-flag_buf_ptr + tb-used, flag, space);
tb-used += space;
-   spin_unlock_irqrestore(buf-lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -317,26 +293,18 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
 int

Re: [next-20130118] Analyzing a call-trace reproducible with pm_test/freezer [ X86|RCU|TTY|EXT4FS|JBD2|PM related? ]

[Ilya Zykov]

Hello!
I don't expert, but
maybe it can help.

I test with:
  while echo mem  /sys/power/state; do sleep 2; done
in one X-terminal, in other I trying playing with keyboard.
(Without playing all right I use ext3.)
And sometimes:

WARNING: at drivers/tty/tty_buffer.c:427 flush_to_ldisc+0x52/0x192()
Hardware name: P5K Premium
tty is NULL
Restarting tasks ... 
Modules linked in: fuse autofs4 af_packet ipt_REJECT nf_conntrack_ipv4 
nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT xt_tcpudp nf_conntrack_ipv6 
nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables x_tables ipv6 
binfmt_misc uinput hid_microsoft hid_generic usbkbd usbmouse usbhid hid 
asus_atk0110 iTCO_wdt iTCO_vendor_support evdev sr_mod cdrom sg ata_generic 
pata_jmicron coretemp kvm_intel kvm microcode pcspkr i2c_i801 lpc_ich mfd_core 
sky2 snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_seq snd_seq_device 
snd_pcm snd_timer snd soundcore snd_page_alloc uhci_hcd nouveau ttm 
drm_kms_helper drm hwmon i2c_algo_bit i2c_core mxm_wmi video wmi button 
intel_agp intel_gtt agpgart
Pid: 263, comm: kworker/1:1 Not tainted 3.8.0-rc3-next-20130118-pm+ #16
Call Trace:
 [8102d984] warn_slowpath_common+0x80/0x98
 [8102da30] warn_slowpath_fmt+0x41/0x43
 [811e5d02] flush_to_ldisc+0x52/0x192
 [8102ee3e] ? console_unlock+0x2f4/0x319
 [81040627] process_one_work+0x1e1/0x2a0
 [810428b3] worker_thread+0x154/0x24e
 [8104275f] ? manage_workers+0x26c/0x26c
 [810460da] kthread+0xb0/0xb8
 [8104602a] ? kthread_freezable_should_stop+0x60/0x60
 [813045ec] ret_from_fork+0x7c/0xb0
 [8104602a] ? kthread_freezable_should_stop+0x60/0x60


Also maybe you will be intresting this:
https://lkml.org/lkml/2012/12/18/368


Cpu model name  : Intel(R) Core(TM)2 Duo CPU E8400  @ 3.00GHz
My .config:
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT=elf64-x86-64
CONFIG_ARCH_DEFCONFIG=arch/x86/configs/x86_64_defconfig
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS=-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=
CONFIG_LOCALVERSION=-pm
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME=(none)
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_AUDIT_LOGINUID_IMMUTABLE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TREE_RCU=y
CONFIG_RCU_STALL_COMMON=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_IKCONFIG=m
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=20
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_PROT_NUMA_PROT_NONE=y
CONFIG_NAMESPACES=y
CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
CONFIG_PID_NS=y
CONFIG_NET_NS=y
CONFIG_UIDGID_CONVERTED=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_SYSCTL=y
CONFIG_ANON_INODES=y
CONFIG_EXPERT=y
CONFIG_HAVE_UID16=y
CONFIG_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_ALL=y
CONFIG_HOTPLUG=y
CONFIG_PRINTK=y
CONFIG_BUG=y

Re: [next-20130118] Analyzing a call-trace reproducible with pm_test/freezer [ X86|RCU|TTY|EXT4FS|JBD2|PM related? ]

[Ilya Zykov]

On 20.01.2013 2:51, Sedat Dilek wrote:
 On Sat, Jan 19, 2013 at 11:43 PM, Ilya Zykov i...@ilyx.ru wrote:
 Hello!
 I don't expert, but
 maybe it can help.

 I test with:
   while echo mem  /sys/power/state; do sleep 2; done
 in one X-terminal, in other I trying playing with keyboard.
 (Without playing all right I use ext3.)
 
 Can you test with the patches listed in [1] (including two TTY patches
 from you!)?
 My base is Linux-Next (next-20130118).
 FYI: My Ubuntu/precise is EXT4FS-formatted.
 
Ok.
but not now I have 3.00 night.
I already use this in my 3.8.0-rc3-next-20130118-pm+.
 tty: Correct tty buffer flush.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

Signed-off-by: Ilya Zykov 
---
 drivers/tty/tty_ioctl.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 8481b29..cc0fc52 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1096,12 +1096,16 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
ld = tty_ldisc_ref_wait(tty);
switch (arg) {
case TCIFLUSH:
-   if (ld && ld->ops->flush_buffer)
+   if (ld && ld->ops->flush_buffer) {
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
break;
case TCIOFLUSH:
-   if (ld && ld->ops->flush_buffer)
+   if (ld && ld->ops->flush_buffer) {
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(buf->head->read = buf->head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov 
---
 drivers/tty/tty_buffer.c |  105 +-
 1 files changed, 29 insertions(+), 76 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 45d9161..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = >buf;
struct tty_buffer *thead;
 
-   while ((thead = buf->head) != NULL) {
-   buf->head = thead->next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf->head == NULL))
+   return;
+   while ((thead = buf->head->next) != NULL) {
+   tty_buffer_free(port, buf->head);
+   buf->head = thead;
}
-   buf->tail = NULL;
+   WARN_ON(buf->head != buf->tail);
+   buf->head->read = buf->head->commit;
 }
 
 /**
@@ -193,20 +196,26 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port->buf.lock
+ *
+ * Locking: Takes tty->port->buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-   struct tty_bufhead *buf = >buf;
+   struct tty_bufhead *buf = >port->buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -218,7 +227,7 @@ static int __tty_buffer_request_room(struct tty_port *port, 
size_t size)
 
if (left < size) {
/* This is the slow path - looking for new buffers to use */
-   if ((n = tty_buffer_find(port, size)) != NULL) {
+   if ((n = tty_buffer_find(tty->port, size)) != NULL) {
if (b != NULL) {
b->next = n;
b->commit = b->used;
@@ -229,31 +238,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
size = left;
}
 
+   spin_unlock_irqrestore(>lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port->buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   struct tty_port *port = tty->port;
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(>buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(>buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -272,26 +259,17 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = >port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(>lock, flags);
-   space = __tty_buffer_request_room(tty->port, goal);
-   tb = buf->tail;
+   int space 

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(buf-head-read = buf-head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/tty_buffer.c |  105 +-
 1 files changed, 29 insertions(+), 76 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 45d9161..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = port-buf;
struct tty_buffer *thead;
 
-   while ((thead = buf-head) != NULL) {
-   buf-head = thead-next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf-head == NULL))
+   return;
+   while ((thead = buf-head-next) != NULL) {
+   tty_buffer_free(port, buf-head);
+   buf-head = thead;
}
-   buf-tail = NULL;
+   WARN_ON(buf-head != buf-tail);
+   buf-head-read = buf-head-commit;
 }
 
 /**
@@ -193,20 +196,26 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port-buf.lock
+ *
+ * Locking: Takes tty-port-buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-   struct tty_bufhead *buf = port-buf;
+   struct tty_bufhead *buf = tty-port-buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(buf-lock, flags);
+
/* OPTIMISATION: We could keep a per tty zero sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -218,7 +227,7 @@ static int __tty_buffer_request_room(struct tty_port *port, 
size_t size)
 
if (left  size) {
/* This is the slow path - looking for new buffers to use */
-   if ((n = tty_buffer_find(port, size)) != NULL) {
+   if ((n = tty_buffer_find(tty-port, size)) != NULL) {
if (b != NULL) {
b-next = n;
b-commit = b-used;
@@ -229,31 +238,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
size = left;
}
 
+   spin_unlock_irqrestore(buf-lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port-buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   struct tty_port *port = tty-port;
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(port-buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(port-buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -272,26 +259,17 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = tty-port-buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(buf-lock, flags);
-   space = __tty_buffer_request_room(tty-port, goal);
-   tb = buf-tail;
+   int space = tty_buffer_request_room(tty, goal);
+   struct tty_buffer *tb = tty-port-buf.tail;
/* If there is no space

[PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix IRQ45: nobody cared'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/tty_ioctl.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 8481b29..cc0fc52 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1096,12 +1096,16 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
ld = tty_ldisc_ref_wait(tty);
switch (arg) {
case TCIFLUSH:
-   if (ld  ld-ops-flush_buffer)
+   if (ld  ld-ops-flush_buffer) {
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
break;
case TCIOFLUSH:
-   if (ld  ld-ops-flush_buffer)
+   if (ld  ld-ops-flush_buffer) {
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

> 
> Ok, I have a ton of patches from you, lots of different threads, patches
> with the same subject, and I don't know what one Alan agreed to.
> 
> So, can you please resend what Alan and you agree should be applied to
> the tree?
> 


I think only two important:
[PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[PATCH] tty: Correct tty buffer flush.
With Alan's ACK from 04.12.2012 for kernel <= 3.7

But for kernel 3.8 I resent this 05.12.2012
[PATCH -next 0/2] tty: Correct tty buffer flush.
[PATCH -next 1/2] tty: Correct tty buffer flush.
[PATCH -next 2/2] tty: Correct tty buffer flush.
The last I resent with new description after Jiri's remark.

If need I can resent.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

 
 Ok, I have a ton of patches from you, lots of different threads, patches
 with the same subject, and I don't know what one Alan agreed to.
 
 So, can you please resend what Alan and you agree should be applied to
 the tree?
 


I think only two important:
[PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[PATCH] tty: Correct tty buffer flush.
With Alan's ACK from 04.12.2012 for kernel = 3.7

But for kernel 3.8 I resent this 05.12.2012
[PATCH -next 0/2] tty: Correct tty buffer flush.
[PATCH -next 1/2] tty: Correct tty buffer flush.
[PATCH -next 2/2] tty: Correct tty buffer flush.
The last I resent with new description after Jiri's remark.

If need I can resent.




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix unreasonable write toward closed pty.

[Ilya Zykov]

On 19.12.2012 23:10, Alan Cox wrote:
>> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
>> index a82b399..1ce1362 100644
>> --- a/drivers/tty/pty.c
>> +++ b/drivers/tty/pty.c
>> @@ -116,6 +116,8 @@ static int pty_space(struct tty_struct *to)
>>  
>>  static int pty_write(struct tty_struct *tty, const unsigned char *buf, int 
>> c)
>>  {
>> +if (test_bit(TTY_OTHER_CLOSED, >flags))
>> +return -EIO;
> 
> The flag can change between the test and ny further code being executed ?
> 
> Alan
> 

Yes, if I understand you correctly, but this is no matter here,
because ldisc's layer will wait, flush this data later.
Here, only beginning stage of tty_close.
This is safe later stage from unnecessary data.

Ilya

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

When we are opening ptmx, we have closed pts, by description.
Now only if we open and after close all pts' descriptions, pty_close() sets
this bit correctly

Signed-off-by: Ilya Zykov 
---
 drivers/tty/pty.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 1ce1362..7b69307 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -659,6 +659,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
retval = ptm_driver->ops->open(tty, filp);
if (retval)
goto err_release;
+   set_bit(TTY_OTHER_CLOSED, >flags); /* THE SLAVE STILL CLOSED */
 
tty_unlock(tty);
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Fix unreasonable write toward closed pty.

[Ilya Zykov]

We should not write toward the closed pty. 
Now it happens, if one side close last file descriptor,
and other side in this moment write to it.
It also prevents scheduling unnecessary work.

Signed-off-by: Ilya Zykov 
---
 drivers/tty/pty.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index a82b399..1ce1362 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -116,6 +116,8 @@ static int pty_space(struct tty_struct *to)
 
 static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
 {
+   if (test_bit(TTY_OTHER_CLOSED, >flags))
+   return -EIO;
struct tty_struct *to = tty->link;
 
if (tty->stopped)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Fix unreasonable write toward closed pty.

[Ilya Zykov]

We should not write toward the closed pty. 
Now it happens, if one side close last file descriptor,
and other side in this moment write to it.
It also prevents scheduling unnecessary work.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/pty.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index a82b399..1ce1362 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -116,6 +116,8 @@ static int pty_space(struct tty_struct *to)
 
 static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
 {
+   if (test_bit(TTY_OTHER_CLOSED, tty-flags))
+   return -EIO;
struct tty_struct *to = tty-link;
 
if (tty-stopped)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

When we are opening ptmx, we have closed pts, by description.
Now only if we open and after close all pts' descriptions, pty_close() sets
this bit correctly

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/pty.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 1ce1362..7b69307 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -659,6 +659,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
retval = ptm_driver-ops-open(tty, filp);
if (retval)
goto err_release;
+   set_bit(TTY_OTHER_CLOSED, tty-flags); /* THE SLAVE STILL CLOSED */
 
tty_unlock(tty);
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Fix unreasonable write toward closed pty.

[Ilya Zykov]

On 19.12.2012 23:10, Alan Cox wrote:
 diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
 index a82b399..1ce1362 100644
 --- a/drivers/tty/pty.c
 +++ b/drivers/tty/pty.c
 @@ -116,6 +116,8 @@ static int pty_space(struct tty_struct *to)
  
  static int pty_write(struct tty_struct *tty, const unsigned char *buf, int 
 c)
  {
 +if (test_bit(TTY_OTHER_CLOSED, tty-flags))
 +return -EIO;
 
 The flag can change between the test and ny further code being executed ?
 
 Alan
 

Yes, if I understand you correctly, but this is no matter here,
because ldisc's layer will wait, flush this data later.
Here, only beginning stage of tty_close.
This is safe later stage from unnecessary data.

Ilya

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 00/11] tty: Fix buffer work access-after-free

[Ilya Zykov]

Stress test for tty. :)
You can use this program for debug new tty changes.
Use with caution.

In any case(with/without Peter's patches) I have BUG():

BUG: unable to handle kernel NULL pointer dereference at 004c
IP: [] devpts_pty_kill+0x17/0x81
PGD 48696067 PUD a79c5067 PMD 0 
Oops:  [#1] SMP 
Pid: 7877, comm: a.out Tainted: P   O 3.7.0-next-20121214-tty.1+ #9 
System manufacturer P5K Premium/P5K Premium
RIP: 0010:[]  [] devpts_pty_kill+0x17/0x81
RSP: 0018:8800484a3aa8  EFLAGS: 00010292
RAX: 88012f0385a0 RBX:  RCX: 
RDX:  RSI: 0282 RDI: 
RBP: 8800484a3ac8 R08:  R09: 880046f26d40
R10: 81426ec8 R11: 0246 R12: 8800486a6c00
R13: 8800484c7180 R14: 880046ec4890 R15: fffb
FS:  7f9a64345700() GS:88012fd0() knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2: 004c CR3: a7a01000 CR4: 000407e0
DR0:  DR1:  DR2: 
DR3:  DR6: 0ff0 DR7: 0400
Process a.out (pid: 7877, threadinfo 8800484a2000, task 88007576d220)
Stack:
 8801 88004854a400 8800486a6c00 8800484c7180
 8800484a3ae8 811e0c1b 8800484c7180 88004854a400
 8800484a3bd8 811d83aa 880046f26d78 0009
Call Trace:
 [] pty_close+0x123/0x14f
 [] tty_release+0x17a/0x53d
 [] ? __mutex_unlock_slowpath+0x15/0x39
 [] ptmx_open+0x12c/0x161
 [] chrdev_open+0x12a/0x14b
 [] ? cdev_put+0x23/0x23
 [] do_dentry_open+0x170/0x217
 [] finish_open+0x34/0x40
 [] do_last+0x8c4/0xa72
 [] ? path_init+0xd6/0x2fe
 [] path_openat+0xcb/0x363
 [] ? __dequeue_entity+0x2e/0x33
 [] do_filp_open+0x38/0x84
 [] ? __alloc_fd+0x51/0x110
 [] do_sys_open+0x6d/0xff
 [] sys_open+0x1c/0x1e
 [] system_call_fastpath+0x16/0x1b
Code: 08 02 00 00 48 89 c7 e8 6c f3 fb ff 5b 4c 89 e0 41 5c c9 c3 55 48 89 e5 
41 55 41 54 53 48 89 fb 48 83 ec 08 48 8b 05 80 43 71 00 <81> 7f 4c 02 00 50 00 
48 8b 40 08 4c 8b 60 60 75 04 0f 0b eb fe 
RIP  [] devpts_pty_kill+0x17/0x81
 RSP 
CR2: 004c


Without Peter's patches I have WARN():

7388:Dec 18 22:43:12 bm kernel: [ 2484.054010] WARNING: at 
drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x52/0x1b2()
7389-Dec 18 22:43:12 bm kernel: [ 2484.054011] Hardware name: P5K Premium
7390-Dec 18 22:43:12 bm kernel: [ 2484.054012] tty is NULL
7392-Dec 18 22:43:12 bm kernel: [ 2484.054042] Pid: 3030, comm: kworker/0:0 
Tainted: PW  O 3.7.0-next-20121214-tty.1 #8
7393-Dec 18 22:43:12 bm kernel: [ 2484.054044] Call Trace:
7394-Dec 18 22:43:12 bm kernel: [ 2484.054047]  [] 
warn_slowpath_common+0x80/0x98
7395-Dec 18 22:43:12 bm kernel: [ 2484.054049]  [] 
warn_slowpath_fmt+0x41/0x43
7396-Dec 18 22:43:12 bm kernel: [ 2484.054051]  [] 
flush_to_ldisc+0x52/0x1b2
7397-Dec 18 22:43:12 bm kernel: [ 2484.054053]  [] ? 
__schedule+0x5d1/0x613
7398-Dec 18 22:43:12 bm kernel: [ 2484.054056]  [] 
process_one_work+0x1c1/0x279
7399-Dec 18 22:43:12 bm kernel: [ 2484.054058]  [] ? 
tty_buffer_free_all+0x4d/0x4d
7400-Dec 18 22:43:12 bm kernel: [ 2484.054060]  [] 
worker_thread+0x154/0x24e
7401-Dec 18 22:43:12 bm kernel: [ 2484.054062]  [] ? 
manage_workers+0x26c/0x26c
7402-Dec 18 22:43:12 bm kernel: [ 2484.054064]  [] 
kthread+0xb0/0xb8
7403-Dec 18 22:43:12 bm kernel: [ 2484.054066]  [] ? 
kthread_parkme+0x1f/0x1f
7404-Dec 18 22:43:12 bm kernel: [ 2484.054068]  [] 
ret_from_fork+0x7c/0xb0
7405-Dec 18 22:43:12 bm kernel: [ 2484.054070]  [] ? 
kthread_parkme+0x1f/0x1f


With Peter's patches I have WARN():

WARNING: at drivers/tty/n_tty.c:160 n_tty_set_room+0xe7/0xf8()
Hardware name: P5K Premium
scheduling buffer work for halted ldisc
Pid: 3127, comm: a.out Tainted: PW  O 3.7.0-next-20121214-tty.1+ #9
Call Trace:
 [] warn_slowpath_common+0x80/0x98
 [] warn_slowpath_fmt+0x41/0x43
 [] n_tty_set_room+0xe7/0xf8
 [] reset_buffer_flags+0xad/0xb6
 [] n_tty_open+0xca/0x11f
 [] tty_ldisc_open+0x4e/0x5f
 [] tty_ldisc_hangup+0x1f5/0x292
 [] ? fasync_helper+0x22/0x6c
 [] __tty_hangup+0x102/0x30e
 [] ? d_delete+0x12d/0x136
 [] tty_vhangup+0x9/0xb
 [] pty_close+0x143/0x14f
 [] tty_release+0x17a/0x53d
 [] ? __wake_up+0x3f/0x48
 [] ? fsnotify+0x21d/0x244
 [] __fput+0xf9/0x1bd
 [] fput+0x9/0xb
 [] task_work_run+0x80/0x98
 [] do_notify_resume+0x58/0x69
 [] int_signal+0x12/0x17


---
/*
 *  stress_test_tty.c
 *
 *  Created on: Dec, 2012
 *  Copyright (C) 2012  Ilya Zykov
 *
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or F

Re: [PATCH v2 00/11] tty: Fix buffer work access-after-free

[Ilya Zykov]

 [811d83aa] tty_release+0x17a/0x53d
 [8104b9f7] ? __wake_up+0x3f/0x48
 [810efb55] ? fsnotify+0x21d/0x244
 [810c4bc5] __fput+0xf9/0x1bd
 [810c4ccf] fput+0x9/0xb
 [81041cd4] task_work_run+0x80/0x98
 [810025bd] do_notify_resume+0x58/0x69
 [812ee8da] int_signal+0x12/0x17


---
/*
 *  stress_test_tty.c
 *
 *  Created on: Dec, 2012
 *  Copyright (C) 2012  Ilya Zykov
 *
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program.  If not, see http://www.gnu.org/licenses/.
 */

#include stdio.h
#include fcntl.h
#include sys/ioctl.h
#include termios.h
#include stdlib.h
#include pthread.h
#include signal.h

#define BUF_SIZE 2
#define ERROR_EXIT_CODE 1
#define parent child_id

static int
mfd=-1, sfd=-1, parent=1;

static pthread_t
pth_id;

static char
pty_name[24], buf[]={ '1', '\n' };


static void
pty_exit(int ret, char * exit_message){
if (sfd = 0) close(sfd);
if (mfd = 0) close(mfd);
printf(%s %s %s exit. \n,exit_message?exit_message:,
ret?Error:Normal, parent?parent:child);
exit(ret);
}

static void
pty_init(void){
int ptn;
if( (mfd=open(/dev/ptmx, O_RDWR ))  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't open /dev/ptmx. \n);
if (ioctl(mfd, TIOCGPTN, ptn)  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't get pty number. \n);
snprintf(pty_name, sizeof(pty_name), /dev/pts/%d, ptn);
//printf(Slave pty name = %s.\n,pty_name);
ptn=0;
if (ioctl(mfd, TIOCSPTLCK, ptn)  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't unlock pty slave. \n);
if ( (sfd=open(pty_name, O_RDWR ))  0 )
pty_exit(ERROR_EXIT_CODE, Couldn't open pty slave. \n);
}

static void *
pty_thread_open(void * arg) {
static char ret[]=Thread open has been created.\n;
printf(ret);
do {
close(open(pty_name, O_RDWR ));
} while(1);
return ret;
}

static void *
pty_thread_read(void * arg) {
static char ret[]=Thread read has been created.\n;
printf(ret);
do {
read(sfd, buf, BUF_SIZE);
} while(1);
return ret;
}

static void *
pty_thread_write(void * arg) {
static char ret[]=Thread write has been created.\n;
printf(ret);
do {
write(mfd, buf, BUF_SIZE);
} while(1);
return ret;
}
int main(int argc,char *argv[]) {
pty_init();
child_id=fork();
if(parent) {
sleep(100);
kill(child_id, SIGINT);
pty_exit(0,Parent normal exit\n);
}
pthread_create(pth_id, NULL, pty_thread_open, 0);
/*  For WARNINGS.
pthread_create(pth_id, NULL, pty_thread_write, 0);
pthread_create(pth_id, NULL, pty_thread_read, 0);
*/
do {
close(sfd);
close(mfd);
pty_init();
} while(1);
return 0;
}

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free

[Ilya Zykov]

On 04.12.2012 11:07, Peter Hurley wrote:
> 
> The most common cause stems from the n_tty_close() path spuriously
> scheduling buffer work, when the ldisc has already been halted.
> This is fixed in 'tty: Don't reschedule buffer work while closing'

Thank you, 
very useful.
Fix this:

WARNING: at drivers/tty/tty_buffer.c:429 flush_to_ldisc+0x52/0x192()
Hardware name: P5K Premium
tty is NULL
...
Pid: 1394, comm: kworker/0:2 Tainted: PW  O 
3.7.0-rc8-next-20121210-debtty.1+ #4
Call Trace:
 [] warn_slowpath_common+0x80/0x98
 [] warn_slowpath_fmt+0x41/0x43
 [] flush_to_ldisc+0x52/0x192
 [] ? __schedule+0x5dd/0x60c
 [] process_one_work+0x1c1/0x279
 [] ? tty_buffer_free_all+0x4d/0x4d
 [] worker_thread+0x154/0x24e
 [] ? manage_workers+0x26c/0x26c
 [] kthread+0xb0/0xb8
 [] ? kthread_parkme+0x1f/0x1f
 [] ret_from_fork+0x7c/0xb0
 [] ? kthread_parkme+0x1f/0x1f

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free

[Ilya Zykov]

On 04.12.2012 11:07, Peter Hurley wrote:
 
 The most common cause stems from the n_tty_close() path spuriously
 scheduling buffer work, when the ldisc has already been halted.
 This is fixed in 'tty: Don't reschedule buffer work while closing'

Thank you, 
very useful.
Fix this:

WARNING: at drivers/tty/tty_buffer.c:429 flush_to_ldisc+0x52/0x192()
Hardware name: P5K Premium
tty is NULL
...
Pid: 1394, comm: kworker/0:2 Tainted: PW  O 
3.7.0-rc8-next-20121210-debtty.1+ #4
Call Trace:
 [8102d2f8] warn_slowpath_common+0x80/0x98
 [8102d3a4] warn_slowpath_fmt+0x41/0x43
 [8119c53a] flush_to_ldisc+0x52/0x192
 [812594bd] ? __schedule+0x5dd/0x60c
 [8103f146] process_one_work+0x1c1/0x279
 [8119c4e8] ? tty_buffer_free_all+0x4d/0x4d
 [8104104b] worker_thread+0x154/0x24e
 [81040ef7] ? manage_workers+0x26c/0x26c
 [810446ef] kthread+0xb0/0xb8
 [8104463f] ? kthread_parkme+0x1f/0x1f
 [8125f7ec] ret_from_fork+0x7c/0xb0
 [8104463f] ? kthread_parkme+0x1f/0x1f

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 10.12.2012 3:08, Rafael J. Wysocki wrote:
> On Sunday, December 09, 2012 09:17:04 PM Ilya Zykov wrote:
>> On 05.12.2012 22:53, Ilya Zykov wrote:
>>> What do I do wrong?
>>>
>>> After: modprobe cpufreq_ondemand
>>> I have:
>>>
>>> WARNING: Error inserting freq_table 
>>> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
>>>  Unknown symbol in module, or unknown parameter (see dmesg)
>>> FATAL: Error inserting cpufreq_ondemand 
>>> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
>>>  Unknown symbol in module, or unknown parameter (see dmesg)
>>>
>>> dmesg:
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> __cpufreq_driver_getavg (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> sysfs_create_group (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> sysfs_remove_group (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> __cpufreq_driver_target (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> get_cpu_idle_time_us (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> delayed_work_timer_fn (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> get_cpu_iowait_time_us (err 0)
>>>
>>> cat /proc/kallsyms |grep freq_dr
>>> 814976e0 T __cpufreq_driver_target
>>> 814987e0 T __cpufreq_driver_getavg
>>> 81498850 T cpufreq_driver_target
>>> 81881650 r __ksymtab___cpufreq_driver_getavg
>>> 81881660 r __ksymtab___cpufreq_driver_target
>>> 81883b40 r __ksymtab_cpufreq_driver_target
>>> 81894290 r __kcrctab___cpufreq_driver_getavg
>>> 81894298 r __kcrctab___cpufreq_driver_target
>>> 81895508 r __kcrctab_cpufreq_driver_target
>>> 818b3080 r __kstrtab___cpufreq_driver_getavg
>>> 818b3098 r __kstrtab_cpufreq_driver_target
>>> 818b30ae r __kstrtab___cpufreq_driver_target
>>> 81e240c8 b cpufreq_driver_lock
>>> 81e240d0 b cpufreq_driver
>>> a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]
>>
>> I managed to fix:
>>
>> git checkout 16642a2e7be23bb -- drivers/cpufreq
>> git checkout 250f6715a4112d668 -- include/linux/cpufreq.h
> 
> Can you please send your .config?
> 
> Rafael
> 
> 

gcc --version
gcc (GCC) 4.4.6 20120305 (Red Hat 4.4.6-4)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 10.12.2012 3:08, Rafael J. Wysocki wrote:
> On Sunday, December 09, 2012 09:17:04 PM Ilya Zykov wrote:
>> On 05.12.2012 22:53, Ilya Zykov wrote:
>>> What do I do wrong?
>>>
>>> After: modprobe cpufreq_ondemand
>>> I have:
>>>
>>> WARNING: Error inserting freq_table 
>>> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
>>>  Unknown symbol in module, or unknown parameter (see dmesg)
>>> FATAL: Error inserting cpufreq_ondemand 
>>> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
>>>  Unknown symbol in module, or unknown parameter (see dmesg)
>>>
>>> dmesg:
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> __cpufreq_driver_getavg (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> sysfs_create_group (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> sysfs_remove_group (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> __cpufreq_driver_target (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> get_cpu_idle_time_us (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> delayed_work_timer_fn (err 0)
>>> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
>>> get_cpu_iowait_time_us (err 0)
>>>
>>> cat /proc/kallsyms |grep freq_dr
>>> 814976e0 T __cpufreq_driver_target
>>> 814987e0 T __cpufreq_driver_getavg
>>> 81498850 T cpufreq_driver_target
>>> 81881650 r __ksymtab___cpufreq_driver_getavg
>>> 81881660 r __ksymtab___cpufreq_driver_target
>>> 81883b40 r __ksymtab_cpufreq_driver_target
>>> 81894290 r __kcrctab___cpufreq_driver_getavg
>>> 81894298 r __kcrctab___cpufreq_driver_target
>>> 81895508 r __kcrctab_cpufreq_driver_target
>>> 818b3080 r __kstrtab___cpufreq_driver_getavg
>>> 818b3098 r __kstrtab_cpufreq_driver_target
>>> 818b30ae r __kstrtab___cpufreq_driver_target
>>> 81e240c8 b cpufreq_driver_lock
>>> 81e240d0 b cpufreq_driver
>>> a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]
>>
>> I managed to fix:
>>
>> git checkout 16642a2e7be23bb -- drivers/cpufreq
>> git checkout 250f6715a4112d668 -- include/linux/cpufreq.h
> 
> Can you please send your .config?
> 
> Rafael
> 
> 


CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_LOCALVERSION="-debtty.1"
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENER

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 05.12.2012 22:53, Ilya Zykov wrote:
> What do I do wrong?
> 
> After: modprobe cpufreq_ondemand
> I have:
> 
> WARNING: Error inserting freq_table 
> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
>  Unknown symbol in module, or unknown parameter (see dmesg)
> FATAL: Error inserting cpufreq_ondemand 
> (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
>  Unknown symbol in module, or unknown parameter (see dmesg)
> 
> dmesg:
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> __cpufreq_driver_getavg (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> sysfs_create_group (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> sysfs_remove_group (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> __cpufreq_driver_target (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> get_cpu_idle_time_us (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> delayed_work_timer_fn (err 0)
> Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
> get_cpu_iowait_time_us (err 0)
> 
> cat /proc/kallsyms |grep freq_dr
> 814976e0 T __cpufreq_driver_target
> 814987e0 T __cpufreq_driver_getavg
> 81498850 T cpufreq_driver_target
> 81881650 r __ksymtab___cpufreq_driver_getavg
> 81881660 r __ksymtab___cpufreq_driver_target
> 81883b40 r __ksymtab_cpufreq_driver_target
> 81894290 r __kcrctab___cpufreq_driver_getavg
> 81894298 r __kcrctab___cpufreq_driver_target
> 81895508 r __kcrctab_cpufreq_driver_target
> 818b3080 r __kstrtab___cpufreq_driver_getavg
> 818b3098 r __kstrtab_cpufreq_driver_target
> 818b30ae r __kstrtab___cpufreq_driver_target
> 81e240c8 b cpufreq_driver_lock
> 81e240d0 b cpufreq_driver
> a0c42000 d acpi_cpufreq_driver[acpi_cpufreq]

I managed to fix:

git checkout 16642a2e7be23bb -- drivers/cpufreq
git checkout 250f6715a4112d668 -- include/linux/cpufreq.h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 05.12.2012 22:53, Ilya Zykov wrote:
 What do I do wrong?
 
 After: modprobe cpufreq_ondemand
 I have:
 
 WARNING: Error inserting freq_table 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)
 FATAL: Error inserting cpufreq_ondemand 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)
 
 dmesg:
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_getavg (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_create_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_remove_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_target (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_idle_time_us (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 delayed_work_timer_fn (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_iowait_time_us (err 0)
 
 cat /proc/kallsyms |grep freq_dr
 814976e0 T __cpufreq_driver_target
 814987e0 T __cpufreq_driver_getavg
 81498850 T cpufreq_driver_target
 81881650 r __ksymtab___cpufreq_driver_getavg
 81881660 r __ksymtab___cpufreq_driver_target
 81883b40 r __ksymtab_cpufreq_driver_target
 81894290 r __kcrctab___cpufreq_driver_getavg
 81894298 r __kcrctab___cpufreq_driver_target
 81895508 r __kcrctab_cpufreq_driver_target
 818b3080 r __kstrtab___cpufreq_driver_getavg
 818b3098 r __kstrtab_cpufreq_driver_target
 818b30ae r __kstrtab___cpufreq_driver_target
 81e240c8 b cpufreq_driver_lock
 81e240d0 b cpufreq_driver
 a0c42000 d acpi_cpufreq_driver[acpi_cpufreq]

I managed to fix:

git checkout 16642a2e7be23bb -- drivers/cpufreq
git checkout 250f6715a4112d668 -- include/linux/cpufreq.h

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 10.12.2012 3:08, Rafael J. Wysocki wrote:
 On Sunday, December 09, 2012 09:17:04 PM Ilya Zykov wrote:
 On 05.12.2012 22:53, Ilya Zykov wrote:
 What do I do wrong?

 After: modprobe cpufreq_ondemand
 I have:

 WARNING: Error inserting freq_table 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)
 FATAL: Error inserting cpufreq_ondemand 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)

 dmesg:
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_getavg (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_create_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_remove_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_target (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_idle_time_us (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 delayed_work_timer_fn (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_iowait_time_us (err 0)

 cat /proc/kallsyms |grep freq_dr
 814976e0 T __cpufreq_driver_target
 814987e0 T __cpufreq_driver_getavg
 81498850 T cpufreq_driver_target
 81881650 r __ksymtab___cpufreq_driver_getavg
 81881660 r __ksymtab___cpufreq_driver_target
 81883b40 r __ksymtab_cpufreq_driver_target
 81894290 r __kcrctab___cpufreq_driver_getavg
 81894298 r __kcrctab___cpufreq_driver_target
 81895508 r __kcrctab_cpufreq_driver_target
 818b3080 r __kstrtab___cpufreq_driver_getavg
 818b3098 r __kstrtab_cpufreq_driver_target
 818b30ae r __kstrtab___cpufreq_driver_target
 81e240c8 b cpufreq_driver_lock
 81e240d0 b cpufreq_driver
 a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

 I managed to fix:

 git checkout 16642a2e7be23bb -- drivers/cpufreq
 git checkout 250f6715a4112d668 -- include/linux/cpufreq.h
 
 Can you please send your .config?
 
 Rafael
 
 


CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT=elf64-x86-64
CONFIG_ARCH_DEFCONFIG=arch/x86/configs/x86_64_defconfig
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS=-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=
CONFIG_LOCALVERSION=-debtty.1
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME=(none)
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TREE_RCU=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_LOG_BUF_SHIFT=20
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y
CONFIG_NAMESPACES=y
CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
CONFIG_PID_NS=y
CONFIG_NET_NS=y
CONFIG_UIDGID_CONVERTED=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE=
CONFIG_RD_GZIP=y

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

On 10.12.2012 3:08, Rafael J. Wysocki wrote:
 On Sunday, December 09, 2012 09:17:04 PM Ilya Zykov wrote:
 On 05.12.2012 22:53, Ilya Zykov wrote:
 What do I do wrong?

 After: modprobe cpufreq_ondemand
 I have:

 WARNING: Error inserting freq_table 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)
 FATAL: Error inserting cpufreq_ondemand 
 (/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
  Unknown symbol in module, or unknown parameter (see dmesg)

 dmesg:
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_getavg (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_create_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 sysfs_remove_group (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 __cpufreq_driver_target (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_idle_time_us (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 delayed_work_timer_fn (err 0)
 Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
 get_cpu_iowait_time_us (err 0)

 cat /proc/kallsyms |grep freq_dr
 814976e0 T __cpufreq_driver_target
 814987e0 T __cpufreq_driver_getavg
 81498850 T cpufreq_driver_target
 81881650 r __ksymtab___cpufreq_driver_getavg
 81881660 r __ksymtab___cpufreq_driver_target
 81883b40 r __ksymtab_cpufreq_driver_target
 81894290 r __kcrctab___cpufreq_driver_getavg
 81894298 r __kcrctab___cpufreq_driver_target
 81895508 r __kcrctab_cpufreq_driver_target
 818b3080 r __kstrtab___cpufreq_driver_getavg
 818b3098 r __kstrtab_cpufreq_driver_target
 818b30ae r __kstrtab___cpufreq_driver_target
 81e240c8 b cpufreq_driver_lock
 81e240d0 b cpufreq_driver
 a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

 I managed to fix:

 git checkout 16642a2e7be23bb -- drivers/cpufreq
 git checkout 250f6715a4112d668 -- include/linux/cpufreq.h
 
 Can you please send your .config?
 
 Rafael
 
 

gcc --version
gcc (GCC) 4.4.6 20120305 (Red Hat 4.4.6-4)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

What do I do wrong?

After: modprobe cpufreq_ondemand
I have:

WARNING: Error inserting freq_table 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)
FATAL: Error inserting cpufreq_ondemand 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)

dmesg:
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_getavg (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_create_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_remove_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_target (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_idle_time_us (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
delayed_work_timer_fn (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_iowait_time_us (err 0)

cat /proc/kallsyms |grep freq_dr
814976e0 T __cpufreq_driver_target
814987e0 T __cpufreq_driver_getavg
81498850 T cpufreq_driver_target
81881650 r __ksymtab___cpufreq_driver_getavg
81881660 r __ksymtab___cpufreq_driver_target
81883b40 r __ksymtab_cpufreq_driver_target
81894290 r __kcrctab___cpufreq_driver_getavg
81894298 r __kcrctab___cpufreq_driver_target
81895508 r __kcrctab_cpufreq_driver_target
818b3080 r __kstrtab___cpufreq_driver_getavg
818b3098 r __kstrtab_cpufreq_driver_target
818b30ae r __kstrtab___cpufreq_driver_target
81e240c8 b cpufreq_driver_lock
81e240d0 b cpufreq_driver
a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

lsmod |grep cpufr
acpi_cpufreq   18066  1 
freq_table 14199  1 acpi_cpufreq
mperf  12668  1 acpi_cpufreq

.config

CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_GPIO=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_LOCALVERSION="-ttybuf.1"
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y

CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

CONFIG_TREE_RCU=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_LOG_BUF_SHIFT=19
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CPUSETS=y
CONFIG_PROC_PID_CPUSET=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_RESOURCE_COUNTERS=y
CONFIG_CGROUP_PERF=y

[BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

What do I do wrong?

After: modprobe cpufreq_ondemand
I have:

WARNING: Error inserting freq_table 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)
FATAL: Error inserting cpufreq_ondemand 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)

dmesg:
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_getavg (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_create_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_remove_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_target (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_idle_time_us (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
delayed_work_timer_fn (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_iowait_time_us (err 0)

cat /proc/kallsyms |grep freq_dr
814976e0 T __cpufreq_driver_target
814987e0 T __cpufreq_driver_getavg
81498850 T cpufreq_driver_target
81881650 r __ksymtab___cpufreq_driver_getavg
81881660 r __ksymtab___cpufreq_driver_target
81883b40 r __ksymtab_cpufreq_driver_target
81894290 r __kcrctab___cpufreq_driver_getavg
81894298 r __kcrctab___cpufreq_driver_target
81895508 r __kcrctab_cpufreq_driver_target
818b3080 r __kstrtab___cpufreq_driver_getavg
818b3098 r __kstrtab_cpufreq_driver_target
818b30ae r __kstrtab___cpufreq_driver_target
81e240c8 b cpufreq_driver_lock
81e240d0 b cpufreq_driver
a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

lsmod |grep cpufr
acpi_cpufreq   18066  1 
freq_table 14199  1 acpi_cpufreq
mperf  12668  1 acpi_cpufreq

.config

CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_GPIO=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_LOCALVERSION="-ttybuf.1"
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y

CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

CONFIG_TREE_RCU=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_LOG_BUF_SHIFT=19
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CPUSETS=y
CONFIG_PROC_PID_CPUSET=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_RESOURCE_COUNTERS=y
CONFIG_CGROUP_PERF=y

[PATCH -next 2/2] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 7602df8..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = >buf;
struct tty_buffer *thead;
 
-   while ((thead = buf->head) != NULL) {
-   buf->head = thead->next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf->head == NULL))
+   return;
+   while ((thead = buf->head->next) != NULL) {
+   tty_buffer_free(port, buf->head);
+   buf->head = thead;
}
-   buf->tail = NULL;
+   WARN_ON(buf->head != buf->tail);
+   buf->head->read = buf->head->commit;
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -next 2/2] tty: Correct tty buffer flush.

[Ilya Zykov]

tty: Correct tty buffer flush.

Signed-off-by: Ilya Zykov 
---
 drivers/tty/tty_buffer.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 7602df8..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = >buf;
struct tty_buffer *thead;
 
-   while ((thead = buf->head) != NULL) {
-   buf->head = thead->next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf->head == NULL))
+   return;
+   while ((thead = buf->head->next) != NULL) {
+   tty_buffer_free(port, buf->head);
+   buf->head = thead;
}
-   buf->tail = NULL;
+   WARN_ON(buf->head != buf->tail);
+   buf->head->read = buf->head->commit;
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -next 1/2] tty: Correct tty buffer flush.

[Ilya Zykov]

Revert: tty: hold lock across tty buffer finding and buffer filling

Signed-off-by: Ilya Zykov 
---
 drivers/tty/tty_buffer.c |   94 +++---
 1 files changed, 22 insertions(+), 72 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 45d9161..7602df8 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -193,20 +193,26 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port->buf.lock
+ *
+ * Locking: Takes tty->port->buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-   struct tty_bufhead *buf = >buf;
+   struct tty_bufhead *buf = >port->buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -218,7 +224,7 @@ static int __tty_buffer_request_room(struct tty_port *port, 
size_t size)
 
if (left < size) {
/* This is the slow path - looking for new buffers to use */
-   if ((n = tty_buffer_find(port, size)) != NULL) {
+   if ((n = tty_buffer_find(tty->port, size)) != NULL) {
if (b != NULL) {
b->next = n;
b->commit = b->used;
@@ -229,31 +235,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
size = left;
}
 
+   spin_unlock_irqrestore(>lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port->buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   struct tty_port *port = tty->port;
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(>buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(>buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -272,26 +256,17 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = >port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(>lock, flags);
-   space = __tty_buffer_request_room(tty->port, goal);
-   tb = buf->tail;
+   int space = tty_buffer_request_room(tty, goal);
+   struct tty_buffer *tb = tty->port->buf.tail;
/* If there is no space then tb may be NULL */
-   if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(>lock, flags);
+   if (unlikely(space == 0))
break;
-   }
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
-   spin_unlock_irqrestore(>lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -318,26 +293,17 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
 int tty_insert_flip_string_flags(struct tty_struct *tty,
const unsigned char *chars, const char *flags, size_t size)
 {
-   struct tty_bufhead *buf = >port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long __flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(>lo

[PATCH -next 0/2] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -next 0/2] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty-buf.head-read = tty-buf.head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov i...@ilyx.ru
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -next 1/2] tty: Correct tty buffer flush.

[Ilya Zykov]

Revert: tty: hold lock across tty buffer finding and buffer filling

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/tty_buffer.c |   94 +++---
 1 files changed, 22 insertions(+), 72 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 45d9161..7602df8 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -193,20 +193,26 @@ static struct tty_buffer *tty_buffer_find(struct tty_port 
*port, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold port-buf.lock
+ *
+ * Locking: Takes tty-port-buf.lock
  */
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
-   struct tty_bufhead *buf = port-buf;
+   struct tty_bufhead *buf = tty-port-buf;
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(buf-lock, flags);
+
/* OPTIMISATION: We could keep a per tty zero sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -218,7 +224,7 @@ static int __tty_buffer_request_room(struct tty_port *port, 
size_t size)
 
if (left  size) {
/* This is the slow path - looking for new buffers to use */
-   if ((n = tty_buffer_find(port, size)) != NULL) {
+   if ((n = tty_buffer_find(tty-port, size)) != NULL) {
if (b != NULL) {
b-next = n;
b-commit = b-used;
@@ -229,31 +235,9 @@ static int __tty_buffer_request_room(struct tty_port 
*port, size_t size)
size = left;
}
 
+   spin_unlock_irqrestore(buf-lock, flags);
return size;
 }
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port-buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   struct tty_port *port = tty-port;
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(port-buf.lock, flags);
-   length = __tty_buffer_request_room(port, size);
-   spin_unlock_irqrestore(port-buf.lock, flags);
-   return length;
-}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -272,26 +256,17 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
 {
-   struct tty_bufhead *buf = tty-port-buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(buf-lock, flags);
-   space = __tty_buffer_request_room(tty-port, goal);
-   tb = buf-tail;
+   int space = tty_buffer_request_room(tty, goal);
+   struct tty_buffer *tb = tty-port-buf.tail;
/* If there is no space then tb may be NULL */
-   if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(buf-lock, flags);
+   if (unlikely(space == 0))
break;
-   }
memcpy(tb-char_buf_ptr + tb-used, chars, space);
memset(tb-flag_buf_ptr + tb-used, flag, space);
tb-used += space;
-   spin_unlock_irqrestore(buf-lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -318,26 +293,17 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
 int tty_insert_flip_string_flags(struct tty_struct *tty,
const unsigned char *chars, const char *flags, size_t size)
 {
-   struct tty_bufhead *buf = tty-port-buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long __flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(buf-lock, __flags);
-   space = __tty_buffer_request_room(tty-port, goal

[PATCH -next 2/2] tty: Correct tty buffer flush.

[Ilya Zykov]

tty: Correct tty buffer flush.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/tty_buffer.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 7602df8..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = port-buf;
struct tty_buffer *thead;
 
-   while ((thead = buf-head) != NULL) {
-   buf-head = thead-next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf-head == NULL))
+   return;
+   while ((thead = buf-head-next) != NULL) {
+   tty_buffer_free(port, buf-head);
+   buf-head = thead;
}
-   buf-tail = NULL;
+   WARN_ON(buf-head != buf-tail);
+   buf-head-read = buf-head-commit;
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -next 2/2] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty-buf.head-read = tty-buf.head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 7602df8..8ad 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = port-buf;
struct tty_buffer *thead;
 
-   while ((thead = buf-head) != NULL) {
-   buf-head = thead-next;
-   tty_buffer_free(port, thead);
+   if (unlikely(buf-head == NULL))
+   return;
+   while ((thead = buf-head-next) != NULL) {
+   tty_buffer_free(port, buf-head);
+   buf-head = thead;
}
-   buf-tail = NULL;
+   WARN_ON(buf-head != buf-tail);
+   buf-head-read = buf-head-commit;
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

What do I do wrong?

After: modprobe cpufreq_ondemand
I have:

WARNING: Error inserting freq_table 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)
FATAL: Error inserting cpufreq_ondemand 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)

dmesg:
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_getavg (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_create_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_remove_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_target (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_idle_time_us (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
delayed_work_timer_fn (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_iowait_time_us (err 0)

cat /proc/kallsyms |grep freq_dr
814976e0 T __cpufreq_driver_target
814987e0 T __cpufreq_driver_getavg
81498850 T cpufreq_driver_target
81881650 r __ksymtab___cpufreq_driver_getavg
81881660 r __ksymtab___cpufreq_driver_target
81883b40 r __ksymtab_cpufreq_driver_target
81894290 r __kcrctab___cpufreq_driver_getavg
81894298 r __kcrctab___cpufreq_driver_target
81895508 r __kcrctab_cpufreq_driver_target
818b3080 r __kstrtab___cpufreq_driver_getavg
818b3098 r __kstrtab_cpufreq_driver_target
818b30ae r __kstrtab___cpufreq_driver_target
81e240c8 b cpufreq_driver_lock
81e240d0 b cpufreq_driver
a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

lsmod |grep cpufr
acpi_cpufreq   18066  1 
freq_table 14199  1 acpi_cpufreq
mperf  12668  1 acpi_cpufreq

.config

CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT=elf64-x86-64
CONFIG_ARCH_DEFCONFIG=arch/x86/configs/x86_64_defconfig
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_GPIO=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS=-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=
CONFIG_LOCALVERSION=-ttybuf.1
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME=(none)
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y

CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

CONFIG_TREE_RCU=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_LOG_BUF_SHIFT=19
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CPUSETS=y
CONFIG_PROC_PID_CPUSET=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_RESOURCE_COUNTERS=y
CONFIG_CGROUP_PERF=y
CONFIG_CGROUP_SCHED=y

Re: [BUG -next] cpufreq: cpufreq_governor.

[Ilya Zykov]

What do I do wrong?

After: modprobe cpufreq_ondemand
I have:

WARNING: Error inserting freq_table 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/freq_table.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)
FATAL: Error inserting cpufreq_ondemand 
(/lib/modules/3.7.0-rc8-next-20121205-ttybuf.1+/kernel/drivers/cpufreq/cpufreq_ondemand.ko):
 Unknown symbol in module, or unknown parameter (see dmesg)

dmesg:
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_getavg (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_create_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol sysfs_remove_group 
(err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
__cpufreq_driver_target (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_idle_time_us (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
delayed_work_timer_fn (err 0)
Dec  5 22:26:11 bm kernel: cpufreq_governor: Unknown symbol 
get_cpu_iowait_time_us (err 0)

cat /proc/kallsyms |grep freq_dr
814976e0 T __cpufreq_driver_target
814987e0 T __cpufreq_driver_getavg
81498850 T cpufreq_driver_target
81881650 r __ksymtab___cpufreq_driver_getavg
81881660 r __ksymtab___cpufreq_driver_target
81883b40 r __ksymtab_cpufreq_driver_target
81894290 r __kcrctab___cpufreq_driver_getavg
81894298 r __kcrctab___cpufreq_driver_target
81895508 r __kcrctab_cpufreq_driver_target
818b3080 r __kstrtab___cpufreq_driver_getavg
818b3098 r __kstrtab_cpufreq_driver_target
818b30ae r __kstrtab___cpufreq_driver_target
81e240c8 b cpufreq_driver_lock
81e240d0 b cpufreq_driver
a0c42000 d acpi_cpufreq_driver  [acpi_cpufreq]

lsmod |grep cpufr
acpi_cpufreq   18066  1 
freq_table 14199  1 acpi_cpufreq
mperf  12668  1 acpi_cpufreq

.config

CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT=elf64-x86-64
CONFIG_ARCH_DEFCONFIG=arch/x86/configs/x86_64_defconfig
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_GPIO=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_DEFAULT_IDLE=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CPU_AUTOPROBE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS=-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11
CONFIG_ARCH_CPU_PROBE_RELEASE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

CONFIG_EXPERIMENTAL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=
CONFIG_LOCALVERSION=-ttybuf.1
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
CONFIG_DEFAULT_HOSTNAME=(none)
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
CONFIG_HAVE_GENERIC_HARDIRQS=y

CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BUILD=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

CONFIG_TREE_RCU=y
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
CONFIG_LOG_BUF_SHIFT=19
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CPUSETS=y
CONFIG_PROC_PID_CPUSET=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_RESOURCE_COUNTERS=y
CONFIG_CGROUP_PERF=y
CONFIG_CGROUP_SCHED=y

Re: flush_to_ldisc accesses tty after free (was: [PATCH 21/21] TTY: move tty buffers to tty_port)

[Ilya Zykov]

On 01.12.2012 18:59, Peter Hurley wrote:
> (cc'ing Ilya Zykov  because the test jig below is based on
> his test program from https://lkml.org/lkml/2012/11/29/368 -- just want
> to give credit where credit is due)
> 
> On Fri, 2012-11-30 at 18:52 -0500, Sasha Levin wrote:
>>
>> Still reproducible, I'm still seeing this with the patch above applied:
>>
>> [ 1315.419759] [ cut here ]
>> [ 1315.420611] WARNING: at drivers/tty/tty_buffer.c:476 
>> flush_to_ldisc+0x60/0x200()
>> [ 1315.423098] tty is NULL
> 
> Thanks for sticking with this Sasha. Finally me too.
> 
> ---
> [   88.331234] WARNING: at 
> /home/peter/src/kernels/next/drivers/tty/tty_buffer.c:435 
> flush_to_ldisc+0x194/0x1d0()
> [   88.334505] Hardware name: Bochs
> [   88.335618] tty is bad=-1
> [   88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth 
> snd_hda_intel snd_hda_codec snd_hwdep
> parport_pc ppdev snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq 
> snd_timer snd_seq_device mac_hid
> psmouse snd i2c_piix4 soundcore snd_page_alloc microcode serio_raw 
> virtio_balloon lp parport floppy 8139too 8139cp
> [   88.345272] Pid: 39, comm: kworker/1:1 Tainted: GW
> 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug
> [   88.347736] Call Trace:
> [   88.349024]  [] warn_slowpath_common+0x7f/0xc0
> [   88.350383]  [] warn_slowpath_fmt+0x46/0x50
> [   88.351745]  [] flush_to_ldisc+0x194/0x1d0
> [   88.353047]  [] ? _raw_spin_unlock_irq+0x21/0x50
> [   88.354190]  [] ? finish_task_switch+0x49/0xe0
> [   88.355436]  [] process_one_work+0x121/0x490
> [   88.357674]  [] ? __tty_buffer_flush+0x90/0x90
> [   88.358954]  [] worker_thread+0x164/0x3e0
> [   88.360247]  [] ? manage_workers+0x120/0x120
> [   88.361282]  [] kthread+0xc0/0xd0
> [   88.362284]  [] ? cmos_do_probe+0x2eb/0x3bf
> [   88.363391]  [] ? flush_kthread_worker+0xb0/0xb0
> [   88.364797]  [] ret_from_fork+0x7c/0xb0
> [   88.366087]  [] ? flush_kthread_worker+0xb0/0xb0
> [   88.367266] ---[ end trace 453a7c9f38fbfec0 ]---
> 
> 
> I figured out how to make this reproduce easily. The test jig at the end
> of this email will generate this multiple times a second.
> 
> The test creates a pty pair and spawns a child which writes to the slave
> pts, while the parent waits for the first write and then abruptly closes
> the master ptm and kills the child. (Just in case, I'd only run the jig
> in a disposable vm. Obviously, the vm needs multiple cores and extra pty
> serial devices ;)
> 
>>From instrumenting the tty_release() path, it's clear that tty_buffer
> work is still scheduled even after tty_release_ldisc() has run. For
> example, with this patch I get the warning below it.
> 
> [Further analysis to follow in subsequent mail...]
> 
> --- >% ---
> [PATCH -next] tty: WARN if buffer work racing with tty free
> 
> 
> Signed-off-by: Peter Hurley 
> ---
>  drivers/tty/tty_io.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 1ce50ec..9d53aec 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1511,6 +1511,8 @@ static void queue_release_one_tty(struct kref *kref)
>  {
>   struct tty_struct *tty = container_of(kref, struct tty_struct, kref);
>  
> + WARN_ON(work_pending(>port->buf.work));
> +
>   /* The hangup queue is now free so we can reuse it rather than
>  waste a chunk of memory for each port */
>   INIT_WORK(>hangup_work, release_one_tty);
> 
> /*
>  * pty_thrash.c
>  *
>  * Based on original test jig by Ilya Zykov 
>  */

Yes, ok with me.
Signed-off-by: Ilya Zykov 

> 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #define parent child_id
> 
> static int fd;
> 
> static void error_exit(char *f, ...)
> {
>   va_list va;
> 
>   va_start(va, f);
>   vprintf(f, va);
>   printf(": %s\n", strerror(errno));
>   va_end(va);
> 
>   if (fd >= 0)
>   close(fd);
> 
>   exit(EXIT_FAILURE);
> }
> 
> int main(int argc, char *argv[]) {
>   int parent;
>   char pts_name[24];
>   int ptn, unlock;
> 
> while (1) {
> 
>   fd = open("/dev/ptmx", O_RDWR);
>   if (fd < 0)
>   error_exit("opening pty master");
>   unlock = 0;
>   if (ioctl(fd, TIOCSPTLCK, ) < 0)
>   error_exit("unlocking pty pair");
>   if (ioctl(fd, TIOCGPTN, ) < 0)
>

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov 
---
 drivers/tty/tty_buffer.c |   96 +-
 1 files changed, 27 insertions(+), 69 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 91e326f..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty->buf.head) != NULL) {
-   tty->buf.head = thead->next;
-   tty_buffer_free(tty, thead);
+   if (tty->buf.head == NULL)
+   return;
+   while ((thead = tty->buf.head->next) != NULL) {
+   tty_buffer_free(tty, tty->buf.head);
+   tty->buf.head = thead;
}
-   tty->buf.tail = NULL;
+   WARN_ON(tty->buf.head != tty->buf.tail);
+   tty->buf.head->read = tty->buf.head->commit;
 }
 
 /**
@@ -185,19 +188,25 @@ static struct tty_buffer *tty_buffer_find(struct 
tty_struct *tty, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold tty->buf.lock
+ *
+ * Locking: Takes tty->buf.lock
  */
-static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(>buf.lock, flags);
+
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -219,29 +228,8 @@ static int __tty_buffer_request_room(struct tty_struct 
*tty, size_t size)
size = left;
}
 
-   return size;
-}
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes tty->buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(>buf.lock, flags);
-   length = __tty_buffer_request_room(tty, size);
spin_unlock_irqrestore(>buf.lock, flags);
-   return length;
+   return size;
 }
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
@@ -264,22 +252,14 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct 
*tty,
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(>buf.lock, flags);
-   space = __tty_buffer_request_room(tty, goal);
-   tb = tty->buf.tail;
+   int space = tty_buffer_request_room(tty, goal);
+   struct tty_buffer *tb = tty->buf.tail;
/* If there is no space then tb may be NULL */
-   if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(>buf.lock, flags);
+   if (unlikely(space == 0))
break;
-   }
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
-   spin_unlock_irqrestore(>buf.lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -309,22 +289,14 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
 

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 04.12.2012 12:53, Alan Cox wrote:
>> Main idea here -  we never flash last (struct tty_buffer) in the
>> active buffer. Only data for ldisc. (tty->buf.head->read =
>> tty->buf.head->commit). At that moment driver can collect(write) data
>> in buffer without conflict.
> 
> This one I agree with (sorry it took a while to get to, I wanted to sit
> and think carefully about it as it is subtle and clever)
> 
> Can you generate a single patch which reverts the unneeded locks and
> adds this instead. Add a Signed-off-by and send it to Greg to get it
> into 3.8/3.9 somewhere.
> 
> Thanks
> 
> Alan
> 

OK.
Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 04.12.2012 12:53, Alan Cox wrote:
 Main idea here -  we never flash last (struct tty_buffer) in the
 active buffer. Only data for ldisc. (tty-buf.head-read =
 tty-buf.head-commit). At that moment driver can collect(write) data
 in buffer without conflict.
 
 This one I agree with (sorry it took a while to get to, I wanted to sit
 and think carefully about it as it is subtle and clever)
 
 Can you generate a single patch which reverts the unneeded locks and
 adds this instead. Add a Signed-off-by and send it to Greg to get it
 into 3.8/3.9 somewhere.
 
 Thanks
 
 Alan
 

OK.
Thank you.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never free last (struct tty_buffer) in the 
active buffer.
Only flush the data for ldisc(tty-buf.head-read = tty-buf.head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.

Also revert:
  commit c56a00a165712fd73081f40044b1e64407bb1875
  tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
 drivers/tty/tty_buffer.c |   96 +-
 1 files changed, 27 insertions(+), 69 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 91e326f..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty-buf.head) != NULL) {
-   tty-buf.head = thead-next;
-   tty_buffer_free(tty, thead);
+   if (tty-buf.head == NULL)
+   return;
+   while ((thead = tty-buf.head-next) != NULL) {
+   tty_buffer_free(tty, tty-buf.head);
+   tty-buf.head = thead;
}
-   tty-buf.tail = NULL;
+   WARN_ON(tty-buf.head != tty-buf.tail);
+   tty-buf.head-read = tty-buf.head-commit;
 }
 
 /**
@@ -185,19 +188,25 @@ static struct tty_buffer *tty_buffer_find(struct 
tty_struct *tty, size_t size)
/* Should possibly check if this fails for the largest buffer we
   have queued and recycle that ? */
 }
+
 /**
- * __tty_buffer_request_room   -   grow tty buffer if 
needed
+ * tty_buffer_request_room -   grow tty buffer if needed
  * @tty: tty structure
  * @size: size desired
  *
  * Make at least size bytes of linear space available for the tty
  * buffer. If we fail return the size we managed to find.
- *  Locking: Caller must hold tty-buf.lock
+ *
+ * Locking: Takes tty-buf.lock
  */
-static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 {
struct tty_buffer *b, *n;
int left;
+   unsigned long flags;
+
+   spin_lock_irqsave(tty-buf.lock, flags);
+
/* OPTIMISATION: We could keep a per tty zero sized buffer to
   remove this conditional if its worth it. This would be invisible
   to the callers */
@@ -219,29 +228,8 @@ static int __tty_buffer_request_room(struct tty_struct 
*tty, size_t size)
size = left;
}
 
-   return size;
-}
-
-
-/**
- * tty_buffer_request_room -   grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes tty-buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
-   unsigned long flags;
-   int length;
-
-   spin_lock_irqsave(tty-buf.lock, flags);
-   length = __tty_buffer_request_room(tty, size);
spin_unlock_irqrestore(tty-buf.lock, flags);
-   return length;
+   return size;
 }
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
@@ -264,22 +252,14 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct 
*tty,
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int space;
-   unsigned long flags;
-   struct tty_buffer *tb;
-
-   spin_lock_irqsave(tty-buf.lock, flags);
-   space = __tty_buffer_request_room(tty, goal);
-   tb = tty-buf.tail;
+   int space = tty_buffer_request_room(tty, goal);
+   struct tty_buffer *tb = tty-buf.tail;
/* If there is no space then tb may be NULL */
-   if (unlikely(space == 0)) {
-   spin_unlock_irqrestore(tty-buf.lock, flags);
+   if (unlikely(space == 0))
break;
-   }
memcpy(tb-char_buf_ptr + tb-used, chars, space);
memset(tb-flag_buf_ptr + tb-used, flag, space);
tb-used += space;
-   spin_unlock_irqrestore(tty-buf.lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -309,22 +289,14 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-   int

Re: flush_to_ldisc accesses tty after free (was: [PATCH 21/21] TTY: move tty buffers to tty_port)

[Ilya Zykov]

On 01.12.2012 18:59, Peter Hurley wrote:
 (cc'ing Ilya Zykov i...@ilyx.ru because the test jig below is based on
 his test program from https://lkml.org/lkml/2012/11/29/368 -- just want
 to give credit where credit is due)
 
 On Fri, 2012-11-30 at 18:52 -0500, Sasha Levin wrote:

 Still reproducible, I'm still seeing this with the patch above applied:

 [ 1315.419759] [ cut here ]
 [ 1315.420611] WARNING: at drivers/tty/tty_buffer.c:476 
 flush_to_ldisc+0x60/0x200()
 [ 1315.423098] tty is NULL
 
 Thanks for sticking with this Sasha. Finally me too.
 
 ---
 [   88.331234] WARNING: at 
 /home/peter/src/kernels/next/drivers/tty/tty_buffer.c:435 
 flush_to_ldisc+0x194/0x1d0()
 [   88.334505] Hardware name: Bochs
 [   88.335618] tty is bad=-1
 [   88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth 
 snd_hda_intel snd_hda_codec snd_hwdep
 parport_pc ppdev snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq 
 snd_timer snd_seq_device mac_hid
 psmouse snd i2c_piix4 soundcore snd_page_alloc microcode serio_raw 
 virtio_balloon lp parport floppy 8139too 8139cp
 [   88.345272] Pid: 39, comm: kworker/1:1 Tainted: GW
 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug
 [   88.347736] Call Trace:
 [   88.349024]  [81058aff] warn_slowpath_common+0x7f/0xc0
 [   88.350383]  [81058bf6] warn_slowpath_fmt+0x46/0x50
 [   88.351745]  [81432bd4] flush_to_ldisc+0x194/0x1d0
 [   88.353047]  [816f7fe1] ? _raw_spin_unlock_irq+0x21/0x50
 [   88.354190]  [8108a809] ? finish_task_switch+0x49/0xe0
 [   88.355436]  [81077ad1] process_one_work+0x121/0x490
 [   88.357674]  [81432a40] ? __tty_buffer_flush+0x90/0x90
 [   88.358954]  [81078c84] worker_thread+0x164/0x3e0
 [   88.360247]  [81078b20] ? manage_workers+0x120/0x120
 [   88.361282]  [8107e230] kthread+0xc0/0xd0
 [   88.362284]  [816f] ? cmos_do_probe+0x2eb/0x3bf
 [   88.363391]  [8107e170] ? flush_kthread_worker+0xb0/0xb0
 [   88.364797]  [816fff6c] ret_from_fork+0x7c/0xb0
 [   88.366087]  [8107e170] ? flush_kthread_worker+0xb0/0xb0
 [   88.367266] ---[ end trace 453a7c9f38fbfec0 ]---
 
 
 I figured out how to make this reproduce easily. The test jig at the end
 of this email will generate this multiple times a second.
 
 The test creates a pty pair and spawns a child which writes to the slave
 pts, while the parent waits for the first write and then abruptly closes
 the master ptm and kills the child. (Just in case, I'd only run the jig
 in a disposable vm. Obviously, the vm needs multiple cores and extra pty
 serial devices ;)
 
From instrumenting the tty_release() path, it's clear that tty_buffer
 work is still scheduled even after tty_release_ldisc() has run. For
 example, with this patch I get the warning below it.
 
 [Further analysis to follow in subsequent mail...]
 
 --- % ---
 [PATCH -next] tty: WARN if buffer work racing with tty free
 
 
 Signed-off-by: Peter Hurley pe...@hurleysoftware.com
 ---
  drivers/tty/tty_io.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
 index 1ce50ec..9d53aec 100644
 --- a/drivers/tty/tty_io.c
 +++ b/drivers/tty/tty_io.c
 @@ -1511,6 +1511,8 @@ static void queue_release_one_tty(struct kref *kref)
  {
   struct tty_struct *tty = container_of(kref, struct tty_struct, kref);
  
 + WARN_ON(work_pending(tty-port-buf.work));
 +
   /* The hangup queue is now free so we can reuse it rather than
  waste a chunk of memory for each port */
   INIT_WORK(tty-hangup_work, release_one_tty);
 
 /*
  * pty_thrash.c
  *
  * Based on original test jig by Ilya Zykov i...@ilyx.ru
  */

Yes, ok with me.
Signed-off-by: Ilya Zykov i...@ilyx.ru

 
 #include stdio.h
 #include fcntl.h
 #include sys/ioctl.h
 #include termios.h
 #include stdlib.h
 #include errno.h
 #include string.h
 #include stdarg.h
 #include signal.h
 
 #define parent child_id
 
 static int fd;
 
 static void error_exit(char *f, ...)
 {
   va_list va;
 
   va_start(va, f);
   vprintf(f, va);
   printf(: %s\n, strerror(errno));
   va_end(va);
 
   if (fd = 0)
   close(fd);
 
   exit(EXIT_FAILURE);
 }
 
 int main(int argc, char *argv[]) {
   int parent;
   char pts_name[24];
   int ptn, unlock;
 
 while (1) {
 
   fd = open(/dev/ptmx, O_RDWR);
   if (fd  0)
   error_exit(opening pty master);
   unlock = 0;
   if (ioctl(fd, TIOCSPTLCK, unlock)  0)
   error_exit(unlocking pty pair);
   if (ioctl(fd, TIOCGPTN, ptn)  0)
   error_exit(getting pty #);
   snprintf(pts_name, sizeof(pts_name), /dev/pts/%d, ptn);
 
   child_id = fork();
   if (child_id == -1)
   error_exit(forking child);
 
   if (parent

Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free

[Ilya Zykov]

On 04.12.2012 11:07, Peter Hurley wrote:
> This patch series addresses the causes of flush_to_ldisc accessing
> the tty after freeing.
> 

I think, it is have sense only if you can take effect,
with this patch or something like. I can't. :)

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 2ea176b..f24751d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -170,6 +170,10 @@ struct tty_struct *alloc_tty_struct(void)
return kzalloc(sizeof(struct tty_struct), GFP_KERNEL);
 }

+static void flush_to_ldisc2(struct work_struct *work)
+{
+   printk(KERN_WARNING "Possible intrusion detected.\n");
+}
 /**
  * free_tty_struct -   free a disused tty
  * @tty: tty struct to free
@@ -188,6 +192,8 @@ void free_tty_struct(struct tty_struct *tty)
kfree(tty->write_buf);
tty_buffer_free_all(tty);
tty->magic = 0xDEADDEAD;
+   PREPARE_WORK(>buf.work,flush_to_ldisc2);
+   //memset(tty, 0, sizeof(struct tty_struct));
kfree(tty);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
  Main idea of the patch, this is never release last (struct tty_buffer) in the 
active buffer.
Only flush data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
Test program and bug report you can see:
https://lkml.org/lkml/2012/11/29/368

Cc: sta...@vger.kernel.org
Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty->buf.head) != NULL) {
-   tty->buf.head = thead->next;
-   tty_buffer_free(tty, thead);
+   if (tty->buf.head == NULL)
+   return;
+   while ((thead = tty->buf.head->next) != NULL) {
+   tty_buffer_free(tty, tty->buf.head);
+   tty->buf.head = thead;
}
-   tty->buf.tail = NULL;
+   WARN_ON(tty->buf.head != tty->buf.tail);
+   tty->buf.head->read = tty->buf.head->commit;
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flush.

[Ilya Zykov]

  The root of problem is carelessly zeroing pointer(in function 
__tty_buffer_flush()),
when another thread can use it. It can be cause of NULL pointer dereference.
  Main idea of the patch, this is never release last (struct tty_buffer) in the 
active buffer.
Only flush data for ldisc(tty-buf.head-read = tty-buf.head-commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
Test program and bug report you can see:
https://lkml.org/lkml/2012/11/29/368

Cc: sta...@vger.kernel.org
Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty-buf.head) != NULL) {
-   tty-buf.head = thead-next;
-   tty_buffer_free(tty, thead);
+   if (tty-buf.head == NULL)
+   return;
+   while ((thead = tty-buf.head-next) != NULL) {
+   tty_buffer_free(tty, tty-buf.head);
+   tty-buf.head = thead;
}
-   tty-buf.tail = NULL;
+   WARN_ON(tty-buf.head != tty-buf.tail);
+   tty-buf.head-read = tty-buf.head-commit;
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free

[Ilya Zykov]

On 04.12.2012 11:07, Peter Hurley wrote:
 This patch series addresses the causes of flush_to_ldisc accessing
 the tty after freeing.
 

I think, it is have sense only if you can take effect,
with this patch or something like. I can't. :)

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 2ea176b..f24751d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -170,6 +170,10 @@ struct tty_struct *alloc_tty_struct(void)
return kzalloc(sizeof(struct tty_struct), GFP_KERNEL);
 }

+static void flush_to_ldisc2(struct work_struct *work)
+{
+   printk(KERN_WARNING Possible intrusion detected.\n);
+}
 /**
  * free_tty_struct -   free a disused tty
  * @tty: tty struct to free
@@ -188,6 +192,8 @@ void free_tty_struct(struct tty_struct *tty)
kfree(tty-write_buf);
tty_buffer_free_all(tty);
tty-magic = 0xDEADDEAD;
+   PREPARE_WORK(tty-buf.work,flush_to_ldisc2);
+   //memset(tty, 0, sizeof(struct tty_struct));
kfree(tty);
 }
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
>> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
>> index 6c9b7cd..4f02f9c 100644
>> --- a/drivers/tty/tty_buffer.c
>> +++ b/drivers/tty/tty_buffer.c
>> @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
>>  {
>>  struct tty_buffer *thead;
>>  
>> -while ((thead = tty->buf.head) != NULL) {
>> -tty->buf.head = thead->next;
>> -tty_buffer_free(tty, thead);
>> +if (tty->buf.head == NULL)
>> +return;
>> +while ((thead = tty->buf.head->next) != NULL) {
>> +tty_buffer_free(tty, tty->buf.head);
>> +tty->buf.head = thead;
> 
> This part of the change seems to have no effect at all. There are no
> locks held so there is nothing guaranteeing how the other processors
> views of the order of operations will be affected.
> 
> Alan
> 

Test program for this problem, 
after revert "commit f8f72f047" without "[PATCH] tty: Correct tty buffer 
flushing.", 
it cause of "BUG report"(see below) on SMP system linux-3.7.0-rc7.
Both patches resolve problem for this test. But my patch is more right.IMHO. 
And also fix problem with tty_prepare_string*(). 
Thank you.
--
#include 
#include 
#include 
#include 
#include 

#define BUF_SIZE 4
#define ERROR_EXIT_CODE 1
#define parent child_id

static int
mfd=-1, sfd=-1, parent=1;

static char
pty_name[24];

static void
pty_exit(int ret, char * exit_message){
if (sfd >= 0) close(sfd);
if (mfd >= 0) close(mfd);
printf("%s %s exit. \n %s",ret?"Error":"Normal", 
parent?"parent":"child",
exit_message?exit_message:"");
exit(ret);
}

static void
pty_init(void){
int ptn;
if( (mfd=open("/dev/ptmx", O_RDWR )) < 0 )
pty_exit(ERROR_EXIT_CODE,"Couldn't open /dev/ptmx. \n");
if (ioctl(mfd, TIOCGPTN, ) < 0 )
pty_exit(ERROR_EXIT_CODE,"Couldn't get pty number. \n");
snprintf(pty_name, sizeof(pty_name), "/dev/pts/%d", ptn);
printf("Slave pty name = %s.\n",pty_name);
ptn=0;
if (ioctl(mfd, TIOCSPTLCK, ) < 0 )
pty_exit(ERROR_EXIT_CODE,"Couldn't unlock pty slave. \n");
if ( (sfd=open(pty_name, O_RDWR )) < 0 )
pty_exit(ERROR_EXIT_CODE, "Couldn't open pty slave. \n");
}

int main(int argc,char *argv[]) {
pty_init();
char buf[]={ [0 ... BUF_SIZE-1]='1' };

child_id=fork();
do {
if(parent) {
if ( write(mfd, buf, BUF_SIZE) < 0 )
pty_exit(ERROR_EXIT_CODE, "Parent's write() error.\n");
} else { //Child
if ( tcflush(sfd, TCIFLUSH) < 0 )
pty_exit(ERROR_EXIT_CODE, "Child's tcflush() error.\n");
}
}   while(1);
return 0; //Never
}
--

Nov 29 20:42:07 bm kernel: BUG: unable to handle kernel NULL pointer 
dereference at 0018
Nov 29 20:42:07 bm kernel: IP: [] 
tty_insert_flip_string_fixed_flag+0x74/0xd0
Nov 29 20:42:07 bm kernel: PGD 114bc8067 PUD 11149d067 PMD 0 
Nov 29 20:42:07 bm kernel: Oops:  [#1] SMP 
Nov 29 20:42:07 bm kernel: Modules linked in: fuse autofs4 sunrpc 
cpufreq_ondemand acpi_cpufreq freq_table mperf ipt_REJECT nf_conntrack_ipv4 
nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 
nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 dm_mirror 
dm_region_hash dm_log dm_mod uinput iTCO_wdt iTCO_vendor_support gpio_ich sg 
joydev coretemp kvm_intel kvm microcode pcspkr serio_raw i2c_i801 asus_atk0110 
hwmon lpc_ich sky2 snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_hwdep 
snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc 
nvidia(PO) ext3 jbd mbcache sr_mod cdrom sd_mod crc_t10dif pata_acpi 
ata_generic pata_jmicron ahci libahci
Nov 29 20:42:07 bm kernel: CPU 1 
Nov 29 20:42:07 bm kernel: Pid: 8953, comm: a.out Tainted: P   O 
3.7.0-rc7-1+ #23 System manufacturer P5K Premium/P5K Premium
Nov 29 20:42:07 bm kernel: RIP: 0010:[]  [] 
tty_insert_flip_string_fixed_flag+0x74/0xd0
Nov 29 20:42:07 bm kernel: RSP: 0018:88011dee5d58  EFLAGS: 00010202
Nov 29 20:42:07 bm kernel: RAX: 0004 RBX: 88012934d000 RCX: 
880119cfbc00
Nov 29 20:42:07 bm kernel: RDX: 0246 RSI: 880112de1800 RDI: 
0246
Nov 29 20:42:07 bm kernel: RBP: 88011dee5da8 R08: 880112de1800 R09: 

Nov 29 20:42:07 bm kernel: R10: 7fffcd7827a0 R11: 0246 R12: 

Nov 29 20:42:07 bm kernel: R13: 0004 R14: 0004 R15: 
0004
Nov 29 20:42:07 bm kernel: FS:  7f1e28985700() 
GS:88012fc8() knlGS:
Nov 29 20:42:07 bm kernel: CS:  0010 DS:  ES:  CR0: 8005003b
Nov 29 20:42:07 bm kernel: CR2: 

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
>> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
>> index 6c9b7cd..4f02f9c 100644
>> --- a/drivers/tty/tty_buffer.c
>> +++ b/drivers/tty/tty_buffer.c
>> @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
>>  {
>>  struct tty_buffer *thead;
>>  
>> -while ((thead = tty->buf.head) != NULL) {
>> -tty->buf.head = thead->next;
>> -tty_buffer_free(tty, thead);
>> +if (tty->buf.head == NULL)
>> +return;
>> +while ((thead = tty->buf.head->next) != NULL) {
>> +tty_buffer_free(tty, tty->buf.head);
>> +tty->buf.head = thead;
> 
> This part of the change seems to have no effect at all. There are no
> locks held so there is nothing guaranteeing how the other processors
> views of the order of operations will be affected.
> 
> Alan
> 
Sorry, In you reply not all patch.
Main idea here -  we never flash last (struct tty_buffer) in the active buffer.
Only data for ldisc. (tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.


--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty->buf.head) != NULL) {
-   tty->buf.head = thead->next;
-   tty_buffer_free(tty, thead);
+   if (tty->buf.head == NULL)
+   return;
+   while ((thead = tty->buf.head->next) != NULL) {
+   tty_buffer_free(tty, tty->buf.head);
+   tty->buf.head = thead;
}
-   tty->buf.tail = NULL;
+   WARN_ON(tty->buf.head != tty->buf.tail);
+   tty->buf.head->read = tty->buf.head->commit;
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
>> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
>> index 6c9b7cd..4f02f9c 100644
>> --- a/drivers/tty/tty_buffer.c
>> +++ b/drivers/tty/tty_buffer.c
>> @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
>>  {
>>  struct tty_buffer *thead;
>>  
>> -while ((thead = tty->buf.head) != NULL) {
>> -tty->buf.head = thead->next;
>> -tty_buffer_free(tty, thead);
>> +if (tty->buf.head == NULL)
>> +return;
>> +while ((thead = tty->buf.head->next) != NULL) {
>> +tty_buffer_free(tty, tty->buf.head);
>> +tty->buf.head = thead;
> 
> This part of the change seems to have no effect at all. There are no
> locks held so there is nothing guaranteeing how the other processors
> views of the order of operations will be affected.
> 
> Alan
> 
/**
 *  __tty_buffer_flush  -   flush full tty buffers
 *  @tty: tty to flush
 *
 *  flush all the buffers containing receive data. Caller must
 *  hold the buffer lock and must have ensured no parallel flush to
 *  ldisc is running.
 *
 *  Locking: Caller must hold tty->buf.lock
 */

Please, don't ignore my patch.
Please, Look at it one more time thoroughly.
Before REVERT [PATCH] tty: hold lock across tty buffer finding and buffer 
filling.
Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
 diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
 index 6c9b7cd..4f02f9c 100644
 --- a/drivers/tty/tty_buffer.c
 +++ b/drivers/tty/tty_buffer.c
 @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
  {
  struct tty_buffer *thead;
  
 -while ((thead = tty-buf.head) != NULL) {
 -tty-buf.head = thead-next;
 -tty_buffer_free(tty, thead);
 +if (tty-buf.head == NULL)
 +return;
 +while ((thead = tty-buf.head-next) != NULL) {
 +tty_buffer_free(tty, tty-buf.head);
 +tty-buf.head = thead;
 
 This part of the change seems to have no effect at all. There are no
 locks held so there is nothing guaranteeing how the other processors
 views of the order of operations will be affected.
 
 Alan
 
/**
 *  __tty_buffer_flush  -   flush full tty buffers
 *  @tty: tty to flush
 *
 *  flush all the buffers containing receive data. Caller must
 *  hold the buffer lock and must have ensured no parallel flush to
 *  ldisc is running.
 *
 *  Locking: Caller must hold tty-buf.lock
 */

Please, don't ignore my patch.
Please, Look at it one more time thoroughly.
Before REVERT [PATCH] tty: hold lock across tty buffer finding and buffer 
filling.
Thank you.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
 diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
 index 6c9b7cd..4f02f9c 100644
 --- a/drivers/tty/tty_buffer.c
 +++ b/drivers/tty/tty_buffer.c
 @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
  {
  struct tty_buffer *thead;
  
 -while ((thead = tty-buf.head) != NULL) {
 -tty-buf.head = thead-next;
 -tty_buffer_free(tty, thead);
 +if (tty-buf.head == NULL)
 +return;
 +while ((thead = tty-buf.head-next) != NULL) {
 +tty_buffer_free(tty, tty-buf.head);
 +tty-buf.head = thead;
 
 This part of the change seems to have no effect at all. There are no
 locks held so there is nothing guaranteeing how the other processors
 views of the order of operations will be affected.
 
 Alan
 
Sorry, In you reply not all patch.
Main idea here -  we never flash last (struct tty_buffer) in the active buffer.
Only data for ldisc. (tty-buf.head-read = tty-buf.head-commit).
At that moment driver can collect(write) data in buffer without conflict.


--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty-buf.head) != NULL) {
-   tty-buf.head = thead-next;
-   tty_buffer_free(tty, thead);
+   if (tty-buf.head == NULL)
+   return;
+   while ((thead = tty-buf.head-next) != NULL) {
+   tty_buffer_free(tty, tty-buf.head);
+   tty-buf.head = thead;
}
-   tty-buf.tail = NULL;
+   WARN_ON(tty-buf.head != tty-buf.tail);
+   tty-buf.head-read = tty-buf.head-commit;
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

On 29.11.2012 17:54, Alan Cox wrote:
 diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
 index 6c9b7cd..4f02f9c 100644
 --- a/drivers/tty/tty_buffer.c
 +++ b/drivers/tty/tty_buffer.c
 @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
  {
  struct tty_buffer *thead;
  
 -while ((thead = tty-buf.head) != NULL) {
 -tty-buf.head = thead-next;
 -tty_buffer_free(tty, thead);
 +if (tty-buf.head == NULL)
 +return;
 +while ((thead = tty-buf.head-next) != NULL) {
 +tty_buffer_free(tty, tty-buf.head);
 +tty-buf.head = thead;
 
 This part of the change seems to have no effect at all. There are no
 locks held so there is nothing guaranteeing how the other processors
 views of the order of operations will be affected.
 
 Alan
 

Test program for this problem, 
after revert commit f8f72f047 without [PATCH] tty: Correct tty buffer 
flushing., 
it cause of BUG report(see below) on SMP system linux-3.7.0-rc7.
Both patches resolve problem for this test. But my patch is more right.IMHO. 
And also fix problem with tty_prepare_string*(). 
Thank you.
--
#include stdio.h
#include fcntl.h
#include sys/ioctl.h
#include termios.h
#include stdlib.h

#define BUF_SIZE 4
#define ERROR_EXIT_CODE 1
#define parent child_id

static int
mfd=-1, sfd=-1, parent=1;

static char
pty_name[24];

static void
pty_exit(int ret, char * exit_message){
if (sfd = 0) close(sfd);
if (mfd = 0) close(mfd);
printf(%s %s exit. \n %s,ret?Error:Normal, 
parent?parent:child,
exit_message?exit_message:);
exit(ret);
}

static void
pty_init(void){
int ptn;
if( (mfd=open(/dev/ptmx, O_RDWR ))  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't open /dev/ptmx. \n);
if (ioctl(mfd, TIOCGPTN, ptn)  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't get pty number. \n);
snprintf(pty_name, sizeof(pty_name), /dev/pts/%d, ptn);
printf(Slave pty name = %s.\n,pty_name);
ptn=0;
if (ioctl(mfd, TIOCSPTLCK, ptn)  0 )
pty_exit(ERROR_EXIT_CODE,Couldn't unlock pty slave. \n);
if ( (sfd=open(pty_name, O_RDWR ))  0 )
pty_exit(ERROR_EXIT_CODE, Couldn't open pty slave. \n);
}

int main(int argc,char *argv[]) {
pty_init();
char buf[]={ [0 ... BUF_SIZE-1]='1' };

child_id=fork();
do {
if(parent) {
if ( write(mfd, buf, BUF_SIZE)  0 )
pty_exit(ERROR_EXIT_CODE, Parent's write() error.\n);
} else { //Child
if ( tcflush(sfd, TCIFLUSH)  0 )
pty_exit(ERROR_EXIT_CODE, Child's tcflush() error.\n);
}
}   while(1);
return 0; //Never
}
--

Nov 29 20:42:07 bm kernel: BUG: unable to handle kernel NULL pointer 
dereference at 0018
Nov 29 20:42:07 bm kernel: IP: [81343294] 
tty_insert_flip_string_fixed_flag+0x74/0xd0
Nov 29 20:42:07 bm kernel: PGD 114bc8067 PUD 11149d067 PMD 0 
Nov 29 20:42:07 bm kernel: Oops:  [#1] SMP 
Nov 29 20:42:07 bm kernel: Modules linked in: fuse autofs4 sunrpc 
cpufreq_ondemand acpi_cpufreq freq_table mperf ipt_REJECT nf_conntrack_ipv4 
nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 
nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 dm_mirror 
dm_region_hash dm_log dm_mod uinput iTCO_wdt iTCO_vendor_support gpio_ich sg 
joydev coretemp kvm_intel kvm microcode pcspkr serio_raw i2c_i801 asus_atk0110 
hwmon lpc_ich sky2 snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_hwdep 
snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc 
nvidia(PO) ext3 jbd mbcache sr_mod cdrom sd_mod crc_t10dif pata_acpi 
ata_generic pata_jmicron ahci libahci
Nov 29 20:42:07 bm kernel: CPU 1 
Nov 29 20:42:07 bm kernel: Pid: 8953, comm: a.out Tainted: P   O 
3.7.0-rc7-1+ #23 System manufacturer P5K Premium/P5K Premium
Nov 29 20:42:07 bm kernel: RIP: 0010:[81343294]  [81343294] 
tty_insert_flip_string_fixed_flag+0x74/0xd0
Nov 29 20:42:07 bm kernel: RSP: 0018:88011dee5d58  EFLAGS: 00010202
Nov 29 20:42:07 bm kernel: RAX: 0004 RBX: 88012934d000 RCX: 
880119cfbc00
Nov 29 20:42:07 bm kernel: RDX: 0246 RSI: 880112de1800 RDI: 
0246
Nov 29 20:42:07 bm kernel: RBP: 88011dee5da8 R08: 880112de1800 R09: 

Nov 29 20:42:07 bm kernel: R10: 7fffcd7827a0 R11: 0246 R12: 

Nov 29 20:42:07 bm kernel: R13: 0004 R14: 0004 R15: 
0004
Nov 29 20:42:07 bm kernel: FS:  7f1e28985700() 
GS:88012fc8() knlGS:
Nov 29 20:42:07 bm kernel: CS:  0010 DS:  ES:  CR0: 8005003b
Nov 29 20:42:07 bm kernel: 

Fwd: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

This patch abolish:
[PATCH] tty: hold lock across tty buffer finding and buffer filling.
commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813

The commit f8f72f047 very dirty, has many degradation on SMP systems, because 
take
spinlock at long time, and it doesn't resolve problem with 
tty_prepare_string*().
We lose all advantage from the use of flip buffer. Can't write/read to/from 
buffer
without lock.

The root of problem it use carelessly buffer flushing, then another thread can 
write to it.
This patch resolves this problem and doesn't let lose advantage from flip 
buffer use.

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty->buf.head) != NULL) {
-   tty->buf.head = thead->next;
-   tty_buffer_free(tty, thead);
+   if (tty->buf.head == NULL)
+   return;
+   while ((thead = tty->buf.head->next) != NULL) {
+   tty_buffer_free(tty, tty->buf.head);
+   tty->buf.head = thead;
}
-   tty->buf.tail = NULL;
+   WARN_ON(tty->buf.head != tty->buf.tail);
+   tty->buf.head->read = tty->buf.head->commit;
 }
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Fwd: [PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

This patch abolish:
[PATCH] tty: hold lock across tty buffer finding and buffer filling.
commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813

The commit f8f72f047 very dirty, has many degradation on SMP systems, because 
take
spinlock at long time, and it doesn't resolve problem with 
tty_prepare_string*().
We lose all advantage from the use of flip buffer. Can't write/read to/from 
buffer
without lock.

The root of problem it use carelessly buffer flushing, then another thread can 
write to it.
This patch resolves this problem and doesn't let lose advantage from flip 
buffer use.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty-buf.head) != NULL) {
-   tty-buf.head = thead-next;
-   tty_buffer_free(tty, thead);
+   if (tty-buf.head == NULL)
+   return;
+   while ((thead = tty-buf.head-next) != NULL) {
+   tty_buffer_free(tty, tty-buf.head);
+   tty-buf.head = thead;
}
-   tty-buf.tail = NULL;
+   WARN_ON(tty-buf.head != tty-buf.tail);
+   tty-buf.head-read = tty-buf.head-commit;
 }
 
 /**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

CANCEL - [PATCH] tty: hold lock across tty buffer finding and buffer filling.
commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813

The commit above very dirty, has many degradation on SMP systems, because take
spinlock on long time, and not resolve problem with tty_prepare_string*()(Jiri 
Slaby).
We lose all advantage from the use of flip buffer.

The root of problem it use carelessly buffer flushing, then another thread can 
write to it.
This patch resolve this problem and doesn't let lose advantage from flip buffer 
use.
Before use need REVERT commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813.

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty->buf.head) != NULL) {
-   tty->buf.head = thead->next;
-   tty_buffer_free(tty, thead);
+   if (tty->buf.head == NULL)
+   return;
+   while ((thead = tty->buf.head->next) != NULL) {
+   tty_buffer_free(tty, tty->buf.head);
+   tty->buf.head = thead;
}
-   tty->buf.tail = NULL;
+   WARN_ON(tty->buf.head != tty->buf.tail);
+   tty->buf.head->read = tty->buf.head->commit;
 }
 
 /**

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 27.11.2012 21:24, Greg Kroah-Hartman wrote:
> On Tue, Nov 27, 2012 at 10:14:33AM +0400, Ilya Zykov wrote:
>> Sorry. More correct.
> 
> In what way?  Should I wait for the 6th version?  :)
> 
> thanks,
> 
> greg k-h
> 

No, if only you will accept:
[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

It can be done another way, simple revert:
'tty: fix "IRQ45: nobody cared"'
commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 27.11.2012 21:24, Greg Kroah-Hartman wrote:
 On Tue, Nov 27, 2012 at 10:14:33AM +0400, Ilya Zykov wrote:
 Sorry. More correct.
 
 In what way?  Should I wait for the 6th version?  :)
 
 thanks,
 
 greg k-h
 

No, if only you will accept:
[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

It can be done another way, simple revert:
'tty: fix IRQ45: nobody cared'
commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] tty: Correct tty buffer flushing.

[Ilya Zykov]

CANCEL - [PATCH] tty: hold lock across tty buffer finding and buffer filling.
commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813

The commit above very dirty, has many degradation on SMP systems, because take
spinlock on long time, and not resolve problem with tty_prepare_string*()(Jiri 
Slaby).
We lose all advantage from the use of flip buffer.

The root of problem it use carelessly buffer flushing, then another thread can 
write to it.
This patch resolve this problem and doesn't let lose advantage from flip buffer 
use.
Before use need REVERT commit f8f72f047b96c6c8b13f6e3ba53fa6feb4266813.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6c9b7cd..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 {
struct tty_buffer *thead;
 
-   while ((thead = tty-buf.head) != NULL) {
-   tty-buf.head = thead-next;
-   tty_buffer_free(tty, thead);
+   if (tty-buf.head == NULL)
+   return;
+   while ((thead = tty-buf.head-next) != NULL) {
+   tty_buffer_free(tty, tty-buf.head);
+   tty-buf.head = thead;
}
-   tty-buf.tail = NULL;
+   WARN_ON(tty-buf.head != tty-buf.tail);
+   tty-buf.head-read = tty-buf.head-commit;
 }
 
 /**

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Sorry. More correct.

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..4071a8f 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1096,12 +1096,16 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
ld = tty_ldisc_ref_wait(tty);
switch (arg) {
case TCIFLUSH:
-   if (ld && ld->ops->flush_buffer)
+   if (ld && ld->ops->flush_buffer) {
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
break;
case TCIOFLUSH:
-   if (ld && ld->ops->flush_buffer)
+   if (ld && ld->ops->flush_buffer) {
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

[Ilya Zykov]

Sorry. Correct patch format.

Related bug 'tty: fix "IRQ45: nobody cared"'
Related commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Unfortunately, some drivers indirectly call ldisc's flush_buffer() function 
in own callback function close(). In particularly, by the use of 
tty_ldisc_flush(),
before TTY LAYER calls ldisc's flush_buffer() in the right moment.

1. It disturb the logic of work ldisc's layer.
2. It is simple overhead because we call ldisc's flush_buffer() at least two 
times.

Please, tell me what do you think about this? Can I make the adjustment for 
other drivers?

Signed-off-by: Ilya Zykov 
---
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0fcfd98..d87f353 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1304,7 +1304,7 @@ static void uart_close(struct tty_struct *tty, struct 
file *filp)
uart_shutdown(tty, state);
uart_flush_buffer(tty);
 
-   tty_ldisc_flush(tty);
+   tty_buffer_flush(tty);
 
tty_port_tty_set(port, NULL);
spin_lock_irqsave(>lock, flags);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index d7bdd8d..8a02996 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -428,8 +428,8 @@ int tty_port_close_start(struct tty_port *port,
timeout = 2 * HZ;
schedule_timeout_interruptible(timeout);
}
-   /* Flush the ldisc buffering */
-   tty_ldisc_flush(tty);
+   /* Flush the driver buffering */
+   tty_buffer_flush(tty);
 
/* Drop DTR/RTS if HUPCL is set. This causes any attached modem to
   hang up the line */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Sorry. Correct patch format.

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.


Signed-off-by: Ilya Zykov 
---

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..a038be1 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1098,10 +1098,12 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
case TCIFLUSH:
if (ld && ld->ops->flush_buffer)
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
break;
case TCIOFLUSH:
if (ld && ld->ops->flush_buffer)
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.


Signed-off-by: Ilya Zykov 
---

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..a038be1 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1098,10 +1098,12 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
case TCIFLUSH:
if (ld && ld->ops->flush_buffer)
ld->ops->flush_buffer(tty);
+   tty_unthrottle(tty);
break;
case TCIOFLUSH:
if (ld && ld->ops->flush_buffer)
ld->ops->flush_buffer(tty);
+tty_unthrottle(tty);
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix IRQ45: nobody cared'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.


Signed-off-by: Ilya Zykov i...@ilyx.ru
---

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..a038be1 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1098,10 +1098,12 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
case TCIFLUSH:
if (ld  ld-ops-flush_buffer)
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
break;
case TCIOFLUSH:
if (ld  ld-ops-flush_buffer)
ld-ops-flush_buffer(tty);
+tty_unthrottle(tty);
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Sorry. Correct patch format.

Regression 'tty: fix IRQ45: nobody cared'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.


Signed-off-by: Ilya Zykov i...@ilyx.ru
---

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..a038be1 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1098,10 +1098,12 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
case TCIFLUSH:
if (ld  ld-ops-flush_buffer)
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
break;
case TCIOFLUSH:
if (ld  ld-ops-flush_buffer)
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

[Ilya Zykov]

Sorry. Correct patch format.

Related bug 'tty: fix IRQ45: nobody cared'
Related commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Unfortunately, some drivers indirectly call ldisc's flush_buffer() function 
in own callback function close(). In particularly, by the use of 
tty_ldisc_flush(),
before TTY LAYER calls ldisc's flush_buffer() in the right moment.

1. It disturb the logic of work ldisc's layer.
2. It is simple overhead because we call ldisc's flush_buffer() at least two 
times.

Please, tell me what do you think about this? Can I make the adjustment for 
other drivers?

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0fcfd98..d87f353 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1304,7 +1304,7 @@ static void uart_close(struct tty_struct *tty, struct 
file *filp)
uart_shutdown(tty, state);
uart_flush_buffer(tty);
 
-   tty_ldisc_flush(tty);
+   tty_buffer_flush(tty);
 
tty_port_tty_set(port, NULL);
spin_lock_irqsave(port-lock, flags);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index d7bdd8d..8a02996 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -428,8 +428,8 @@ int tty_port_close_start(struct tty_port *port,
timeout = 2 * HZ;
schedule_timeout_interruptible(timeout);
}
-   /* Flush the ldisc buffering */
-   tty_ldisc_flush(tty);
+   /* Flush the driver buffering */
+   tty_buffer_flush(tty);
 
/* Drop DTR/RTS if HUPCL is set. This causes any attached modem to
   hang up the line */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Sorry. More correct.

Regression 'tty: fix IRQ45: nobody cared'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 12b1fa0..4071a8f 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1096,12 +1096,16 @@ int tty_perform_flush(struct tty_struct *tty, unsigned 
long arg)
ld = tty_ldisc_ref_wait(tty);
switch (arg) {
case TCIFLUSH:
-   if (ld  ld-ops-flush_buffer)
+   if (ld  ld-ops-flush_buffer) {
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
break;
case TCIOFLUSH:
-   if (ld  ld-ops-flush_buffer)
+   if (ld  ld-ops-flush_buffer) {
ld-ops-flush_buffer(tty);
+   tty_unthrottle(tty);
+   }
/* fall through */
case TCOFLUSH:
tty_driver_flush_buffer(tty);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

[Ilya Zykov]

This patch correct tty serial drivers.

Unfortunately, many drivers indirectly call ldisc's flush_buffer() function 
in own callback function close(). Рarticularly, by the use of tty_ldisc_flush(),
before TTY LAYER calls ldisc's flush_buffer() in the right moment.

1. It disturb the logic of work ldisc's layer.
2. It is simple overhead because we call ldisc's flush_buffer() at least two 
times.

Signed-off-by: Ilya Zykov 
---

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a21dc8e..c4a0a6d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1288,7 +1288,7 @@ static void uart_close(struct tty_struct *tty, struct 
file *filp)
uart_shutdown(tty, state);
uart_flush_buffer(tty);
 
-   tty_ldisc_flush(tty);
+   tty_buffer_flush(tty);
 
tty_port_tty_set(port, NULL);
spin_lock_irqsave(>lock, flags);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index bf6e238..c23dd30 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -364,8 +364,8 @@ int tty_port_close_start(struct tty_port *port,
timeout = 2 * HZ;
schedule_timeout_interruptible(timeout);
}
-   /* Flush the ldisc buffering */
-   tty_ldisc_flush(tty);
+   /* Flush the driver buffering */
+   tty_buffer_flush(tty);
 
/* Drop DTR/RTS if HUPCL is set. This causes any attached modem to
   hang up the line */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH]tty: Incorrect use tty_ldisc_flush() in TTY drivers.

[Ilya Zykov]

This patch correct tty serial drivers.

Unfortunately, many drivers indirectly call ldisc's flush_buffer() function 
in own callback function close(). Рarticularly, by the use of tty_ldisc_flush(),
before TTY LAYER calls ldisc's flush_buffer() in the right moment.

1. It disturb the logic of work ldisc's layer.
2. It is simple overhead because we call ldisc's flush_buffer() at least two 
times.

Signed-off-by: Ilya Zykov i...@ilyx.ru
---

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a21dc8e..c4a0a6d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1288,7 +1288,7 @@ static void uart_close(struct tty_struct *tty, struct 
file *filp)
uart_shutdown(tty, state);
uart_flush_buffer(tty);
 
-   tty_ldisc_flush(tty);
+   tty_buffer_flush(tty);
 
tty_port_tty_set(port, NULL);
spin_lock_irqsave(port-lock, flags);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index bf6e238..c23dd30 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -364,8 +364,8 @@ int tty_port_close_start(struct tty_port *port,
timeout = 2 * HZ;
schedule_timeout_interruptible(timeout);
}
-   /* Flush the ldisc buffering */
-   tty_ldisc_flush(tty);
+   /* Flush the driver buffering */
+   tty_buffer_flush(tty);
 
/* Drop DTR/RTS if HUPCL is set. This causes any attached modem to
   hang up the line */

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

About 'tty: fix "IRQ45: nobody cared"':
  We don't call tty_unthrottle() if release last filp - ('tty->count == 0')
In other case it must be safely. 

Unfortunately, many drivers indirectly invoke tty_unthrottle() before TTY LAYER
decremented (tty->count).
  This Patch help us catch bugs in tty's drivers and invoke tty_unthrottle()
in right moment only.

Signed-off-by: Ilya Zykov 
---

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 26f0d0e..f20b44a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -173,6 +173,13 @@ static void reset_buffer_flags(struct tty_struct *tty)
 {
unsigned long flags;
 
+   WARN_ON(tty->drvbug);
+   /*
+* We should not call this method from driver's close function.
+* It will be called by TTY layer later.
+*/
+   if (tty->drvbug)
+   return;
spin_lock_irqsave(>read_lock, flags);
tty->read_head = tty->read_tail = tty->read_cnt = 0;
spin_unlock_irqrestore(>read_lock, flags);
@@ -184,6 +191,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
tty->canon_head = tty->canon_data = tty->erasing = 0;
memset(>read_flags, 0, sizeof tty->read_flags);
n_tty_set_room(tty);
+   check_unthrottle(tty);
 }
 
 /**
@@ -1585,7 +1593,6 @@ static int n_tty_open(struct tty_struct *tty)
return -ENOMEM;
}
reset_buffer_flags(tty);
-   tty_unthrottle(tty);
tty->column = 0;
n_tty_set_termios(tty, NULL);
tty->minimum_to_wake = 1;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..0cd2370 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1649,8 +1649,10 @@ int tty_release(struct inode *inode, struct file *filp)
tty_name(tty, buf), tty->count);
 #endif
 
+   tty->drvbug = 1;
if (tty->ops->close)
tty->ops->close(tty, filp);
+   tty->drvbug = 0;
 
tty_unlock();
/*
diff --git a/include/linux/tty.h b/include/linux/tty.h
index ed1e82e..01455b6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -281,7 +281,7 @@ struct tty_struct {
int count;
struct winsize winsize; /* termios mutex */
unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
-   unsigned char low_latency:1, warned:1;
+   unsigned char low_latency:1, warned:1, drvbug:1;
unsigned char ctrl_status;  /* ctrl_lock */
unsigned int receive_room;  /* Bytes free for queue */



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

Regression 'tty: fix "IRQ45: nobody cared"'
Regression commit 7b292b4bf9a9d6098440d85616d6ca4c608b8304

  Function reset_buffer_flags() also invoked during the ioctl(...,TCFLSH,..). 
At the time of request we can have full buffers and throttled driver too. 
If we don't unthrottle driver, we can get forever throttled driver, because,
after request, we will have empty buffers and throttled driver and 
there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.

About 'tty: fix "IRQ45: nobody cared"':
  We don't call tty_unthrottle() if release last filp - ('tty->count == 0')
In other case it must be safely. 

Unfortunately, many drivers indirectly invoke tty_unthrottle() before TTY LAYER
decremented (tty->count).
  This Patch help us catch bugs in tty's drivers and invoke tty_unthrottle()
in right moment only.

Signed-off-by: Ilya Zykov 
---

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 26f0d0e..f20b44a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -173,6 +173,13 @@ static void reset_buffer_flags(struct tty_struct *tty)
 {
unsigned long flags;
 
+   WARN_ON(tty->drvbug);
+   /*
+* We should not call this method from driver's close function.
+* It will be called by TTY layer later.
+*/
+   if (tty->drvbug)
+   return;
spin_lock_irqsave(>read_lock, flags);
tty->read_head = tty->read_tail = tty->read_cnt = 0;
spin_unlock_irqrestore(>read_lock, flags);
@@ -184,6 +191,7 @@ static void reset_buffer_flags(struct tty_struct *tty)
tty->canon_head = tty->canon_data = tty->erasing = 0;
memset(>read_flags, 0, sizeof tty->read_flags);
n_tty_set_room(tty);
+   check_unthrottle(tty);
 }
 
 /**
@@ -1585,7 +1593,6 @@ static int n_tty_open(struct tty_struct *tty)
return -ENOMEM;
}
reset_buffer_flags(tty);
-   tty_unthrottle(tty);
tty->column = 0;
n_tty_set_termios(tty, NULL);
tty->minimum_to_wake = 1;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..0cd2370 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1649,8 +1649,10 @@ int tty_release(struct inode *inode, struct file *filp)
tty_name(tty, buf), tty->count);
 #endif
 
+   tty->drvbug = 1;
if (tty->ops->close)
tty->ops->close(tty, filp);
+   tty->drvbug = 0;
 
tty_unlock();
/*
diff --git a/include/linux/tty.h b/include/linux/tty.h
index ed1e82e..01455b6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -281,7 +281,7 @@ struct tty_struct {
int count;
struct winsize winsize; /* termios mutex */
unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
-   unsigned char low_latency:1, warned:1;
+   unsigned char low_latency:1, warned:1, drvbug:1;
unsigned char ctrl_status;  /* ctrl_lock */
unsigned int receive_room;  /* Bytes free for queue */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 10:03, Ilya Zykov wrote:

On 22.11.2012 8:29, Ilya Zykov wrote:

On 22.11.2012 4:47, andrew mcgregor wrote:




On 11/22/2012 at 10:39 AM, in message <50ad4a01.7060...@ilyx.ru>,
Ilya Zykov

 wrote:

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to
unthrottle

driver.

It simple reproduce with "pty" pair then one side sleep on
tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do

writers wake up.



So instead of revertng it why not just fix it ? Just add an
argument to
the reset_buffer_flags function to indicate if unthrottling is
permitted.

Alan


Because in my opinion, unthrottling permitted always, except release
last filp (tty->count == 0)


Maybe so, but the patch was there in the first place to resolve an
actual observed bug, where a driver would lock up.  So the behaviour
needs preserved.

Andrew



Maybe it was wrong driver, unfortunately, I didn't find full information
about this bug. As an example, if driver indirectly call
reset_buffer_flags() in driver's close() function it will be before
decrement last (tty->count).




Particularly, many drivers and 'serial_core.c' use tty_ldisc_flush() in
own close() function. tty_ldisc_flush() call reset_buffer_flags()
indirectly.
I think is wrong way use tty_ldisc_flush() in driver's close() function,
because tty layer 'tty_release()' call  tty_ldisc_release() after
decremented (tty->count), and clear all buffers.
We don't care about this in driver. And call ldisc's function.



Sorry, last phrase not correct.
We don't NEED care about this in driver. And call ldisc's function.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 4:47, andrew mcgregor wrote:




On 11/22/2012 at 10:39 AM, in message <50ad4a01.7060...@ilyx.ru>, Ilya Zykov

 wrote:

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle

driver.

It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do

writers wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan


Because in my opinion, unthrottling permitted always, except release
last filp (tty->count == 0)


Maybe so, but the patch was there in the first place to resolve an actual 
observed bug, where a driver would lock up.  So the behaviour needs preserved.

Andrew



Maybe it was wrong driver, unfortunately, I didn't find full information 
about this bug. As an example, if driver indirectly call 
reset_buffer_flags() in driver's close() function it will be before 
decrement last (tty->count).



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan

Because in my opinion, unthrottling permitted always, except release 
last filp (tty->count == 0)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 8:29, Ilya Zykov wrote:

On 22.11.2012 4:47, andrew mcgregor wrote:




On 11/22/2012 at 10:39 AM, in message <50ad4a01.7060...@ilyx.ru>,
Ilya Zykov

 wrote:

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle

driver.

It simple reproduce with "pty" pair then one side sleep on
tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do

writers wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is
permitted.

Alan


Because in my opinion, unthrottling permitted always, except release
last filp (tty->count == 0)


Maybe so, but the patch was there in the first place to resolve an
actual observed bug, where a driver would lock up.  So the behaviour
needs preserved.

Andrew



Maybe it was wrong driver, unfortunately, I didn't find full information
about this bug. As an example, if driver indirectly call
reset_buffer_flags() in driver's close() function it will be before
decrement last (tty->count).




Particularly, many drivers and 'serial_core.c' use tty_ldisc_flush() in
own close() function. tty_ldisc_flush() call reset_buffer_flags() 
indirectly.
I think is wrong way use tty_ldisc_flush() in driver's close() function, 
because tty layer 'tty_release()' call  tty_ldisc_release() after 
decremented (tty->count), and clear all buffers.

We don't care about this in driver. And call ldisc's function.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 9:25, andrew mcgregor wrote:




On 11/22/2012 at 05:29 PM, in message <50adaa26.7080...@ilyx.ru>, Ilya Zykov

 wrote:

On 22.11.2012 4:47, andrew mcgregor wrote:




On 11/22/2012 at 10:39 AM, in message <50ad4a01.7060...@ilyx.ru>, Ilya Zykov

 wrote:

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle

driver.

It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do

writers wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan


Because in my opinion, unthrottling permitted always, except release
last filp (tty->count == 0)


Maybe so, but the patch was there in the first place to resolve an actual

observed bug, where a driver would lock up.  So the behaviour needs
preserved.


Andrew



Maybe it was wrong driver, unfortunately, I didn't find full information
about this bug. As an example, if driver indirectly call
reset_buffer_flags() in driver's close() function it will be before
decrement last (tty->count).


Well, the driver in question was just 8250.c, so you should be able to see that 
the original condition can exist.

Here's the commit message again:

tty: fix "IRQ45: nobody cared"

Unthrottling the TTY during close ends up enabling interrupts
on a device not on the active list, which will never have the
interrupts cleared.  Doctor, it hurts when I do this.


On 6/2/2011 at 01:56 AM, in message <20110601145608.3e586...@bob.linux.org.uk>, Alan 
Cox  wrote:

On Wed, 01 Jun 2011 10:34:07 +1200
"andrew mcgregor"  wrote:

The LKML message
http://kerneltrap.org/mailarchive/linux-kernel/2010/2/25/4541847from
February doesn't seem to have been resolved since.  We struck the
issue, and the patch below (against 2.6.32) fixes it.  Should I
supply a patch against 3.0.0rc?


I think that would be sensible. I don't actually see how you hit it as
the IRQ ought to be masked by then but it's certainly wrong for n_tty
to be calling into check_unthrottle at that point.

So yes please send a patch with a suitable Signed-off-by: line to
linux-serial and cc GregKH  as well.

Alan


Signed-off-by: Andrew McGregor 
Signed-off-by: Greg Kroah-Hartman 

What part of this no longer applies?  I'm happy enough if you can prove that 
this can't happen any more, but otherwise the fix should remain.

Andrew


This patch must help for 8250.c

diff --git a/drivers/tty/serial/serial_core.c 
b/drivers/tty/serial/serial_core.c

index a21dc8e..2e197c3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1288,8 +1288,6 @@ static void uart_close(struct tty_struct *tty, 
struct file *filp)

uart_shutdown(tty, state);
uart_flush_buffer(tty);

-   tty_ldisc_flush(tty);
-
tty_port_tty_set(port, NULL);
spin_lock_irqsave(>lock, flags);
tty->closing = 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle driver.
It simple reproduce with "pty" pair then one side sleep on tty->write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan



Maybe we don't need call reset_buffer_flags() then execute tty_release() 
and (tty->count != 0)? Why in this case we reset buffers?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle driver.
It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do writers 
wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan



Maybe we don't need call reset_buffer_flags() then execute tty_release() 
and (tty-count != 0)? Why in this case we reset buffers?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[Ilya Zykov]

On 22.11.2012 9:25, andrew mcgregor wrote:




On 11/22/2012 at 05:29 PM, in message 50adaa26.7080...@ilyx.ru, Ilya Zykov

i...@ilyx.ru wrote:

On 22.11.2012 4:47, andrew mcgregor wrote:




On 11/22/2012 at 10:39 AM, in message 50ad4a01.7060...@ilyx.ru, Ilya Zykov

i...@ilyx.ru wrote:

On 22.11.2012 1:30, Alan Cox wrote:

Function reset_buffer_flags() also invoked during the
ioctl(...,TCFLSH,..). At the time of request we can have full buffers
and throttled driver too. If we don't unthrottle driver, we can get
forever throttled driver, because after request, we will have
empty buffers and throttled driver and there is no place to unthrottle

driver.

It simple reproduce with pty pair then one side sleep on tty-write_wait,
and other side do ioctl(...,TCFLSH,..). Then there is no place to do

writers wake up.



So instead of revertng it why not just fix it ? Just add an argument to
the reset_buffer_flags function to indicate if unthrottling is permitted.

Alan


Because in my opinion, unthrottling permitted always, except release
last filp (tty-count == 0)


Maybe so, but the patch was there in the first place to resolve an actual

observed bug, where a driver would lock up.  So the behaviour needs
preserved.


Andrew



Maybe it was wrong driver, unfortunately, I didn't find full information
about this bug. As an example, if driver indirectly call
reset_buffer_flags() in driver's close() function it will be before
decrement last (tty-count).


Well, the driver in question was just 8250.c, so you should be able to see that 
the original condition can exist.

Here's the commit message again:

tty: fix IRQ45: nobody cared

Unthrottling the TTY during close ends up enabling interrupts
on a device not on the active list, which will never have the
interrupts cleared.  Doctor, it hurts when I do this.


On 6/2/2011 at 01:56 AM, in message 20110601145608.3e586...@bob.linux.org.uk, Alan 
Cox a...@linux.intel.com wrote:

On Wed, 01 Jun 2011 10:34:07 +1200
andrew mcgregor andrew.mcgre...@alliedtelesis.co.nz wrote:

The LKML message
http://kerneltrap.org/mailarchive/linux-kernel/2010/2/25/4541847from
February doesn't seem to have been resolved since.  We struck the
issue, and the patch below (against 2.6.32) fixes it.  Should I
supply a patch against 3.0.0rc?


I think that would be sensible. I don't actually see how you hit it as
the IRQ ought to be masked by then but it's certainly wrong for n_tty
to be calling into check_unthrottle at that point.

So yes please send a patch with a suitable Signed-off-by: line to
linux-serial and cc GregKH g...@kroah.com as well.

Alan


Signed-off-by: Andrew McGregor andrew.mcgre...@alliedtelesis.co.nz
Signed-off-by: Greg Kroah-Hartman gre...@suse.de

What part of this no longer applies?  I'm happy enough if you can prove that 
this can't happen any more, but otherwise the fix should remain.

Andrew


This patch must help for 8250.c

diff --git a/drivers/tty/serial/serial_core.c 
b/drivers/tty/serial/serial_core.c

index a21dc8e..2e197c3 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1288,8 +1288,6 @@ static void uart_close(struct tty_struct *tty, 
struct file *filp)

uart_shutdown(tty, state);
uart_flush_buffer(tty);

-   tty_ldisc_flush(tty);
-
tty_port_tty_set(port, NULL);
spin_lock_irqsave(port-lock, flags);
tty-closing = 0;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/