Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Thu, 31 Mar 2011 14:38:45 -0700 | Subject | Re: excessive kworker activity when idle. (was Re: vma corruption in today's -git) |
| |
On Thu, Mar 31, 2011 at 9:21 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Mar 31, 2011 at 8:53 AM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> Regardless, I'll put my money where my mouth is, and try to remove the >> crazy re-flush thing. > > Yeah, that doesn't work. The tty does actually lock up when it fills > up. So clearly we actually depended on that reflushing happening. > > That said, I do still think it's the right thing to do to remove that > line, it just means that I need to figure out where the missing flush > is.
Ok, that was unexpected.
So the reason for the need to do that crazy "try to flush from the flush routine" is that in the case of "receive_room" going down to zero (which only happens for n_tty and for the case of switching ldisc's around), if we hit that during flushing, nothing will apparently ever re-start the flushing when receive_room then opens up again.
So instead of having that case re-start the flush, we end up saying "ok, we'll just retry the flush over and over again", and essentially poll for receive_room opening up. No wonder you've seen high CPU use and thousands of calls a second.
The "seen_tail" case doesn't have that issue, because anything that adds a new buffer to the tty list should always be flipping anyway. So this attached patch would seem to work.
Not heavily tested, but the case that I could trivially trigger before doesn't trigger for me any more. And I can't seem to get kworker to waste lots of CPU time any more, but it was kind of hit-and-miss before too, so I don't know how much that's worth..
The locking here is kind of iffy, but otherwise? Comments?
Linus drivers/tty/n_tty.c | 6 ++++++ drivers/tty/tty_buffer.c | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 428f4fe..0ad3288 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -95,6 +95,7 @@ static void n_tty_set_room(struct tty_struct *tty) { /* tty->read_cnt is not read locked ? */ int left = N_TTY_BUF_SIZE - tty->read_cnt - 1; + int old_left; /* * If we are doing input canonicalization, and there are no @@ -104,7 +105,12 @@ static void n_tty_set_room(struct tty_struct *tty) */ if (left <= 0) left = tty->icanon && !tty->canon_data; + old_left = tty->receive_room; tty->receive_room = left; + + /* Did this open up the receive buffer? We may need to flip */ + if (left && !old_left) + schedule_work(&tty->buf.work); } static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty) diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index b945121..f1a7918 100644 --- a/drivers/tty/tty_buffer.c +++ b/drivers/tty/tty_buffer.c @@ -442,10 +442,8 @@ static void flush_to_ldisc(struct work_struct *work) line discipline as we want to empty the queue */ if (test_bit(TTY_FLUSHPENDING, &tty->flags)) break; - if (!tty->receive_room || seen_tail) { - schedule_work(&tty->buf.work); + if (!tty->receive_room || seen_tail) break; - } if (count > tty->receive_room) count = tty->receive_room; char_buf = head->char_buf_ptr + head->read; | |