Re: excessive kworker activity when idle. (was Re: vma corruption in today's -git)

Linus Torvalds
2011-03-31T21:39:10Z

[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]

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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1520 bytes --]

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;