| Message ID | 52277097.8020008@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 4, 2013 at 8:40 PM, Jonathan Austin <jonathan.austin@arm.com> wrote: > 'top' works on ARM with virtio console. I've just done some new testing > and with the serial console emulation and I see the same as you're reporting. > Previously with the 8250 emulation I'd booted to a prompt but didn't actually > test top... > > I'm looking in to fixing this now... Looks like I need to find the right place > from which to call serial8250_flush_tx now that it isn't getting called every tick. > > I've done the following and it works fixes 'top' with serial8250: > -------8<---------- > diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c > index 931067f..a71e68d 100644 > --- a/tools/kvm/hw/serial.c > +++ b/tools/kvm/hw/serial.c > @@ -260,6 +260,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, > dev->lsr &= ~UART_LSR_TEMT; > if (dev->txcnt == FIFO_LEN / 2) > dev->lsr &= ~UART_LSR_THRE; > + serial8250_flush_tx(kvm, dev); > } else { > /* Should never happpen */ > dev->lsr &= ~(UART_LSR_TEMT | UART_LSR_THRE); > > ------------->8----------- > > I guess it's a shame that we'll be printing each character (admittedly the rate will always be > relatively low...) rather than flushing the buffer in a batch. Without a timer, though, I'm > not sure I see a better option - every N chars doesn't seem like a good one to me. > > If you think that looks about right then I'll fold that in to the patch series, probably also > removing the call to serial8250_flush_tx() in serial8250__receive. Yeah, looks good to me and makes top work again. Pekka -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/04/2013 01:48 PM, Pekka Enberg wrote: > On Wed, Sep 4, 2013 at 8:40 PM, Jonathan Austin <jonathan.austin@arm.com> wrote: >> 'top' works on ARM with virtio console. I've just done some new testing >> and with the serial console emulation and I see the same as you're reporting. >> Previously with the 8250 emulation I'd booted to a prompt but didn't actually >> test top... >> >> I'm looking in to fixing this now... Looks like I need to find the right place >> from which to call serial8250_flush_tx now that it isn't getting called every tick. >> >> I've done the following and it works fixes 'top' with serial8250: >> -------8<---------- >> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c >> index 931067f..a71e68d 100644 >> --- a/tools/kvm/hw/serial.c >> +++ b/tools/kvm/hw/serial.c >> @@ -260,6 +260,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, >> dev->lsr &= ~UART_LSR_TEMT; >> if (dev->txcnt == FIFO_LEN / 2) >> dev->lsr &= ~UART_LSR_THRE; >> + serial8250_flush_tx(kvm, dev); >> } else { >> /* Should never happpen */ >> dev->lsr &= ~(UART_LSR_TEMT | UART_LSR_THRE); >> >> ------------->8----------- >> >> I guess it's a shame that we'll be printing each character (admittedly the rate will always be >> relatively low...) rather than flushing the buffer in a batch. Without a timer, though, I'm >> not sure I see a better option - every N chars doesn't seem like a good one to me. >> >> If you think that looks about right then I'll fold that in to the patch series, probably also >> removing the call to serial8250_flush_tx() in serial8250__receive. > > Yeah, looks good to me and makes top work again. We might want to make sure performance isn't hit with stuff that's intensive on the serial console. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sasha, On 04/09/13 19:01, Sasha Levin wrote: > On 09/04/2013 01:48 PM, Pekka Enberg wrote: >> On Wed, Sep 4, 2013 at 8:40 PM, Jonathan Austin <jonathan.austin@arm.com> wrote: >>> 'top' works on ARM with virtio console. I've just done some new testing >>> and with the serial console emulation and I see the same as you're reporting. >>> Previously with the 8250 emulation I'd booted to a prompt but didn't actually >>> test top... >>> >>> I'm looking in to fixing this now... Looks like I need to find the right place >>> from which to call serial8250_flush_tx now that it isn't getting called every tick. >>> >>> I've done the following and it works fixes 'top' with serial8250: >>> -------8<---------- >>> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c >>> index 931067f..a71e68d 100644 >>> --- a/tools/kvm/hw/serial.c >>> +++ b/tools/kvm/hw/serial.c >>> @@ -260,6 +260,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, >>> dev->lsr &= ~UART_LSR_TEMT; >>> if (dev->txcnt == FIFO_LEN / 2) >>> dev->lsr &= ~UART_LSR_THRE; >>> + serial8250_flush_tx(kvm, dev); >>> } else { >>> /* Should never happpen */ >>> dev->lsr &= ~(UART_LSR_TEMT | UART_LSR_THRE); >>> >>> ------------->8----------- >>> >>> I guess it's a shame that we'll be printing each character (admittedly the rate will always be >>> relatively low...) rather than flushing the buffer in a batch. Without a timer, though, I'm >>> not sure I see a better option - every N chars doesn't seem like a good one to me. >>> >>> If you think that looks about right then I'll fold that in to the patch series, probably also >>> removing the call to serial8250_flush_tx() in serial8250__receive. >> >> Yeah, looks good to me and makes top work again. > > We might want to make sure performance isn't hit with stuff that's intensive on the serial console. Indeed, the intention here is very much to reduce overhead... Do you have an idea already of what you'd like to test? I've written a little testcase that just prints an incrementing counter to the console in a tight loop for 5 seconds (I've tested both buffered and unbuffered output). The measure of 'performance' is how high we count in those 5 seconds. These are averages (mean) of 5 runs on x86. ----------------+unbuffered+-buffered-+ native | 36880 | 40354 | ----------------+----------+----------+ lkvm - original | 24302 | 25335 | ----------------+----------+----------+ lkvm - no-tick | 22895 | 28202 | ----------------+----------+----------+ I ran these all on the framebuffer console. I found that the numbers on gnome-terminal seemed to be affected by the activity level of other gui-ish things, and the numbers were different in gnome-terminal and xterm. If you want to see more testing then a suggestion of a way we can take host terminal performance out of the equation would make me more comfortable with the numbers. I do think that as comparisons to each other they're reasonable as they are, though. So at least in this reasonably artificial case it looks like a minor win for buffered output and a minor loss in the unbuffered case. Happy to try out other things if you're interested. Jonny > Thanks, > Sasha > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/05/2013 12:39 PM, Jonathan Austin wrote: > Hi Sasha, > > On 04/09/13 19:01, Sasha Levin wrote: >> On 09/04/2013 01:48 PM, Pekka Enberg wrote: >>> On Wed, Sep 4, 2013 at 8:40 PM, Jonathan Austin <jonathan.austin@arm.com> wrote: >>>> 'top' works on ARM with virtio console. I've just done some new testing >>>> and with the serial console emulation and I see the same as you're reporting. >>>> Previously with the 8250 emulation I'd booted to a prompt but didn't actually >>>> test top... >>>> >>>> I'm looking in to fixing this now... Looks like I need to find the right place >>>> from which to call serial8250_flush_tx now that it isn't getting called every tick. >>>> >>>> I've done the following and it works fixes 'top' with serial8250: >>>> -------8<---------- >>>> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c >>>> index 931067f..a71e68d 100644 >>>> --- a/tools/kvm/hw/serial.c >>>> +++ b/tools/kvm/hw/serial.c >>>> @@ -260,6 +260,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, >>>> dev->lsr &= ~UART_LSR_TEMT; >>>> if (dev->txcnt == FIFO_LEN / 2) >>>> dev->lsr &= ~UART_LSR_THRE; >>>> + serial8250_flush_tx(kvm, dev); >>>> } else { >>>> /* Should never happpen */ >>>> dev->lsr &= ~(UART_LSR_TEMT | UART_LSR_THRE); >>>> >>>> ------------->8----------- >>>> >>>> I guess it's a shame that we'll be printing each character (admittedly the rate will always be >>>> relatively low...) rather than flushing the buffer in a batch. Without a timer, though, I'm >>>> not sure I see a better option - every N chars doesn't seem like a good one to me. >>>> >>>> If you think that looks about right then I'll fold that in to the patch series, probably also >>>> removing the call to serial8250_flush_tx() in serial8250__receive. >>> >>> Yeah, looks good to me and makes top work again. >> >> We might want to make sure performance isn't hit with stuff that's intensive on the serial console. > > Indeed, the intention here is very much to reduce overhead... > > Do you have an idea already of what you'd like to test? > > I've written a little testcase that just prints an incrementing counter to the console in a tight > loop for 5 seconds (I've tested both buffered and unbuffered output). The measure of 'performance' > is how high we count in those 5 seconds. > > These are averages (mean) of 5 runs on x86. > > ----------------+unbuffered+-buffered-+ > native | 36880 | 40354 | > ----------------+----------+----------+ > lkvm - original | 24302 | 25335 | > ----------------+----------+----------+ > lkvm - no-tick | 22895 | 28202 | > ----------------+----------+----------+ > > I ran these all on the framebuffer console. I found that the numbers on gnome-terminal seemed to be > affected by the activity level of other gui-ish things, and the numbers were different in > gnome-terminal and xterm. If you want to see more testing then a suggestion of a way we can take > host terminal performance out of the equation would make me more comfortable with the numbers. I do > think that as comparisons to each other they're reasonable as they are, though. > > So at least in this reasonably artificial case it looks like a minor win for buffered output and a > minor loss in the unbuffered case. > > Happy to try out other things if you're interested. I've played around with it over here. Looks good to me. Thanks! Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c index 931067f..a71e68d 100644 --- a/tools/kvm/hw/serial.c +++ b/tools/kvm/hw/serial.c @@ -260,6 +260,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm *kvm, u16 port, dev->lsr &= ~UART_LSR_TEMT; if (dev->txcnt == FIFO_LEN / 2) dev->lsr &= ~UART_LSR_THRE; + serial8250_flush_tx(kvm, dev); } else { /* Should never happpen */ dev->lsr &= ~(UART_LSR_TEMT | UART_LSR_THRE);