TTY: tty flip buffer change reserve memory strategy.
PATCH 1/1 TTY: tty flip buffer change reserve memory strategy.
Currently, free flip buffer (tty->buf->free) reserve memory for further used, only if driver send to ldisc less 257 bytes in one time.
If driver send more, flip buffer reserve(kmalloc()) and then free(kfree()) every chunk more 256 bytes every time,
even we have in free buffer enough space, because we can't
pass through chunk boundary in free buffer. Also we can't limit reserve memory size. In worse case we will have in free buffer:
(256 * 256 byte chunk) = (256 * 552 ) = 141312 byte unused memory.
This patch allow reserve more than 256 bytes in one time. And have self-regulation chunk size ability(very useful for pty).
Also we can limit reserve memory size, but then,
we will be use kmalloc()-kree() calls on intensive stream from the driver.
drivers/tty/tty_buffer.c +19 -5
@@ -58,7 +58,7 @@ static struct tty_buffer *tty_buffer_all {struct tty_buffer *p; -if (tty->buf.memory_used + size >65536 ) +if (tty->buf.memory_used + size >TTY_BUFFER_MAX )return NULL ; p =kmalloc (sizeof (struct tty_buffer ) +2 * size,GFP_ATOMIC );if (p ==NULL )
@@ -91,10 +91,21 @@ static void tty_buffer_free(struct tty_s tty->buf.memory_used -= b->size;WARN_ON (tty->buf.memory_used <0 ); -if (b->size >=512 ) -kfree (b); -else { - b->next = tty->buf.free; + tty->buf.memory_reserve += b->size; + b->next =NULL ; +if (tty->buf.free !=NULL ) { +struct tty_buffer *p = tty->buf.free; +while (p->next !=NULL ) + p = p->next; + p->next = b; +while (tty->buf.memory_reserve >TTY_BUFFER_RESERVE ) { + p = tty->buf.free; + tty->buf.free = p->next; + tty->buf.memory_reserve -= p->size; +WARN_ON (tty->buf.memory_reserve <0 ); +kfree (p); + } + }else { tty->buf.free = b; } }
@@ -175,6 +186,8 @@ static struct tty_buffer *tty_buffer_fin t->commit =0 ; t->read =0 ; tty->buf.memory_used += t->size; + tty->buf.memory_reserve -= t->size; +WARN_ON (tty->buf.memory_reserve <0 );return t; } tbh = &((*tbh)->next);
@@ -517,6 +530,7 @@ void tty_buffer_init(struct tty_struct * tty->buf.tail =NULL ; tty->buf.free =NULL ; tty->buf.memory_used =0 ; + tty->buf.memory_reserve =0 ;INIT_WORK (&tty->buf.work, flush_to_ldisc); }
include/linux/tty.h +4 -3
@@ -79,7 +79,8 @@ struct tty_buffer {*/ #define TTY_BUFFER_PAGE (((PAGE_SIZE - sizeof(struct tty_buffer)) / 2) & ~0xFF) - +#define TTY_BUFFER_MAX 65536 +#define TTY_BUFFER_RESERVE TTY_BUFFER_MAXstruct tty_bufhead {struct work_struct work;
@@ -87,8 +88,8 @@ struct tty_bufhead {struct tty_buffer *head;/* Queue head */ struct tty_buffer *tail;/* Active buffer */ struct tty_buffer *free;/* Free queue head */ -int memory_used;/* Buffer space used excluding -free queue */ +int memory_used;/* Buffer space used excluding free queue */ +int memory_reserve;/* Free queue space */ };/* * When a break, frame error, or parity error happens, these codes are
Quoted 5 lines
free(kfree()) every chunk more 256 bytes every time,
even we have in free buffer enough space, because we can't pass through chunk boundary in free buffer. Also we can't limit reserve memory size. In worse case we will have in free buffer:
(256 * 256 byte chunk) = (256 * 552 ) = 141312 byte unused memory.
The idea is to trade off between the devices that need long linear blocks (mostly for DMA) and devices that don't care - but many of which pass 1 or 2 bytes/irq so want a common buffer.
Most drivers should not be using tty_reserve_room in the first place.
Quoted 2 lines
Also we can limit reserve memory size, but then,
we will be use kmalloc()-kree() calls on intensive stream from the driver.
kmalloc is very fast anyway so I'd be interested in seeing the drivers suffering and the profile traces of a real example.
I'm also not sure I agree with the maths - we chunk buffers to 256 byte boundaries to avoid getting lots of queued buffers of different sizes. We could in fact I think simplify the code from what we have learned using it and simply treat the 256 byte buffer case as special rather than 256 and 512 as we do now. That would let us remove the more generic support for free buffer sizes which is overcomplex and mean we could simply count the number of 256 byte buffers we have to hand.
Alan
Thank you, for answer.
Quoted 4 lines
kmalloc is very fast anyway so I'd be interested in seeing the drivers suffering and the profile traces of a real example.
Pty use chunk more 256 bytes very often. More program for communicate with pppd use pty master - slave. If often kmalloc()-kfree() calls don't provoke memory fragmentation and flush TLB, I agree it isn't important.
Quoted 7 lines
I'm also not sure I agree with the maths - we chunk buffers to 256 byte boundaries to avoid getting lots of queued buffers of different sizes. We could in fact I think simplify the code from what we have learned using it and simply treat the 256 byte buffer case as special rather than 256 and 512 as we do now. That would let us remove the more generic support for free buffer sizes which is overcomplex and mean we could simply count the number of 256 byte buffers we have to hand.
Now we can't accumulate buffer more than 256 byte.
"if (b->size >= 512) kfree(b);" and our buffer can be only 256, 512 ... . We can simplify tty_buffer_find() for don't look up buffer more 256 byte.
I think my patch very clear, because if stream from driver very slow we will be use only two 256 byte chunk, as before. But with pty, maybe, new drivers, we could use flip buffer more effective.
Ilya.
Quoted 4 lines
Pty use chunk more 256 bytes very often. More program for communicate with pppd use pty master - slave. If often kmalloc()-kfree() calls don't provoke memory fragmentation and flush TLB, I agree it isn't important.
I think any work of this nature ought to involve benchmarking. That would answer the questions better. kmalloc/kfree is a smart allocator so shouldn't cause problems, and it doesn't manipulate address space so shouldn't cause TLB misses.
You want to make these changes, you say they are needed, but what actually talks is real numbers.
Alan