* [PATCH 0/2] console/serial: adjust default TX buffer size @ 2022-06-23 9:08 Roger Pau Monne 2022-06-23 9:08 ` [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig Roger Pau Monne 2022-06-23 9:08 ` [PATCH 2/2] console/serial: bump buffer from 16K to 32K Roger Pau Monne 0 siblings, 2 replies; 10+ messages in thread From: Roger Pau Monne @ 2022-06-23 9:08 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu Hello, First patch moves the setting of the default TX buffer size to Kconfig, and shouldn't be controversial, second patch increases the buffer to 32K. Thanks, Roger. Roger Pau Monne (2): console/serial: set the default transmit buffer size in Kconfig console/serial: bump buffer from 16K to 32K xen/drivers/char/Kconfig | 10 ++++++++++ xen/drivers/char/serial.c | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig 2022-06-23 9:08 [PATCH 0/2] console/serial: adjust default TX buffer size Roger Pau Monne @ 2022-06-23 9:08 ` Roger Pau Monne 2022-06-23 13:30 ` Jan Beulich 2022-06-23 9:08 ` [PATCH 2/2] console/serial: bump buffer from 16K to 32K Roger Pau Monne 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monne @ 2022-06-23 9:08 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu Take the opportunity to convert the variable to read-only after init. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/drivers/char/Kconfig | 10 ++++++++++ xen/drivers/char/serial.c | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig index e5f7b1d8eb..a349d55f18 100644 --- a/xen/drivers/char/Kconfig +++ b/xen/drivers/char/Kconfig @@ -74,3 +74,13 @@ config HAS_EHCI help This selects the USB based EHCI debug port to be used as a UART. If you have an x86 based system with USB, say Y. + +config SERIAL_TX_BUFSIZE + int "Size of the transmit serial buffer" + default 16384 + help + Controls the default size of the transmit buffer (in bytes) used by + the serial driver. Note the value provided will be rounder up to + PAGE_SIZE. + + Default value is 16384 (16KB). diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c index 5ecba0af33..8d375a41e3 100644 --- a/xen/drivers/char/serial.c +++ b/xen/drivers/char/serial.c @@ -16,7 +16,8 @@ /* Never drop characters, even if the async transmit buffer fills. */ /* #define SERIAL_NEVER_DROP_CHARS 1 */ -unsigned int __read_mostly serial_txbufsz = 16384; +unsigned int __ro_after_init serial_txbufsz = ROUNDUP(CONFIG_SERIAL_TX_BUFSIZE, + PAGE_SIZE); size_param("serial_tx_buffer", serial_txbufsz); #define mask_serial_rxbuf_idx(_i) ((_i)&(serial_rxbufsz-1)) -- 2.36.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig 2022-06-23 9:08 ` [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig Roger Pau Monne @ 2022-06-23 13:30 ` Jan Beulich 0 siblings, 0 replies; 10+ messages in thread From: Jan Beulich @ 2022-06-23 13:30 UTC (permalink / raw) To: Roger Pau Monne Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu, xen-devel On 23.06.2022 11:08, Roger Pau Monne wrote: > --- a/xen/drivers/char/Kconfig > +++ b/xen/drivers/char/Kconfig > @@ -74,3 +74,13 @@ config HAS_EHCI > help > This selects the USB based EHCI debug port to be used as a UART. If > you have an x86 based system with USB, say Y. > + > +config SERIAL_TX_BUFSIZE > + int "Size of the transmit serial buffer" > + default 16384 > + help > + Controls the default size of the transmit buffer (in bytes) used by > + the serial driver. Note the value provided will be rounder up to > + PAGE_SIZE. I first wanted to point out the spelling mistake (rounded), but I wonder what good that rounding does and whether this description isn't really misleading: serial_async_transmit() rounds down to a power of two. So likely the value here would better be a log-2 one. > + Default value is 16384 (16KB). Perhaps (16kiB) (albeit the default value would change anyway if the above is followed)? Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-23 9:08 [PATCH 0/2] console/serial: adjust default TX buffer size Roger Pau Monne 2022-06-23 9:08 ` [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig Roger Pau Monne @ 2022-06-23 9:08 ` Roger Pau Monne 2022-06-23 13:32 ` Jan Beulich 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monne @ 2022-06-23 9:08 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer being filled halfway during dom0 boot, and thus a non-trivial chunk of Linux boot messages are dropped. Increasing the buffer to 32K does fix the issue and Linux boot messages are no longer dropped. There's no justification either on why 16K was chosen, and hence bumping to 32K in order to cope with current systems generating output faster does seem appropriate to have a better user experience with the provided defaults. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/drivers/char/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig index a349d55f18..a8ac667ba2 100644 --- a/xen/drivers/char/Kconfig +++ b/xen/drivers/char/Kconfig @@ -77,10 +77,10 @@ config HAS_EHCI config SERIAL_TX_BUFSIZE int "Size of the transmit serial buffer" - default 16384 + default 32768 help Controls the default size of the transmit buffer (in bytes) used by the serial driver. Note the value provided will be rounder up to PAGE_SIZE. - Default value is 16384 (16KB). + Default value is 32768 (32KB). -- 2.36.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-23 9:08 ` [PATCH 2/2] console/serial: bump buffer from 16K to 32K Roger Pau Monne @ 2022-06-23 13:32 ` Jan Beulich 2022-06-29 15:23 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2022-06-23 13:32 UTC (permalink / raw) To: xen-devel Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu, Roger Pau Monne On 23.06.2022 11:08, Roger Pau Monne wrote: > Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer > being filled halfway during dom0 boot, and thus a non-trivial chunk of > Linux boot messages are dropped. > > Increasing the buffer to 32K does fix the issue and Linux boot > messages are no longer dropped. There's no justification either on > why 16K was chosen, and hence bumping to 32K in order to cope with > current systems generating output faster does seem appropriate to have > a better user experience with the provided defaults. Just to record what was part of an earlier discussion: I'm not going to nak such a change, but I think the justification is insufficient: On this same basis someone else could come a few days later and bump to 64k, then 128k, etc. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-23 13:32 ` Jan Beulich @ 2022-06-29 15:23 ` Roger Pau Monné 2022-06-29 16:03 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2022-06-29 15:23 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On Thu, Jun 23, 2022 at 03:32:30PM +0200, Jan Beulich wrote: > On 23.06.2022 11:08, Roger Pau Monne wrote: > > Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer > > being filled halfway during dom0 boot, and thus a non-trivial chunk of > > Linux boot messages are dropped. > > > > Increasing the buffer to 32K does fix the issue and Linux boot > > messages are no longer dropped. There's no justification either on > > why 16K was chosen, and hence bumping to 32K in order to cope with > > current systems generating output faster does seem appropriate to have > > a better user experience with the provided defaults. > > Just to record what was part of an earlier discussion: I'm not going > to nak such a change, but I think the justification is insufficient: > On this same basis someone else could come a few days later and bump > to 64k, then 128k, etc. Indeed, and that would be fine IMO. We should aim to provide defaults that work fine for most situations, and here I don't see what drawback it has to increase the default buffer size from 16kiB to 32kiB, and I would be fine with increasing to 128kiB if that's required for some use case, albeit I have a hard time seeing how we could fill that buffer. If I can ask, what kind of justification you would see fit for granting an increase to the default buffer size? Thanks, Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-29 15:23 ` Roger Pau Monné @ 2022-06-29 16:03 ` Jan Beulich 2022-06-29 16:19 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2022-06-29 16:03 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On 29.06.2022 17:23, Roger Pau Monné wrote: > On Thu, Jun 23, 2022 at 03:32:30PM +0200, Jan Beulich wrote: >> On 23.06.2022 11:08, Roger Pau Monne wrote: >>> Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer >>> being filled halfway during dom0 boot, and thus a non-trivial chunk of >>> Linux boot messages are dropped. >>> >>> Increasing the buffer to 32K does fix the issue and Linux boot >>> messages are no longer dropped. There's no justification either on >>> why 16K was chosen, and hence bumping to 32K in order to cope with >>> current systems generating output faster does seem appropriate to have >>> a better user experience with the provided defaults. >> >> Just to record what was part of an earlier discussion: I'm not going >> to nak such a change, but I think the justification is insufficient: >> On this same basis someone else could come a few days later and bump >> to 64k, then 128k, etc. > > Indeed, and that would be fine IMO. We should aim to provide defaults > that work fine for most situations, and here I don't see what drawback > it has to increase the default buffer size from 16kiB to 32kiB, and > I would be fine with increasing to 128kiB if that's required for some > use case, albeit I have a hard time seeing how we could fill that > buffer. > > If I can ask, what kind of justification you would see fit for > granting an increase to the default buffer size? Making plausible that for a majority of contemporary systems the buffer is not large enough would be one aspect. But then there's imo always going to be an issue: What if non-Linux Dom0 would be far more chatty? What if Linux, down the road, was made less verbose (by default)? What if people expect large enough a buffer to also suffice when Linux runs in e.g. ignore_loglevel mode? We simply can't fit all use cases and at the same time also not go overboard with the default size. That's why there's a way to control this via command line option. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-29 16:03 ` Jan Beulich @ 2022-06-29 16:19 ` Roger Pau Monné 2022-06-29 16:30 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2022-06-29 16:19 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On Wed, Jun 29, 2022 at 06:03:34PM +0200, Jan Beulich wrote: > On 29.06.2022 17:23, Roger Pau Monné wrote: > > On Thu, Jun 23, 2022 at 03:32:30PM +0200, Jan Beulich wrote: > >> On 23.06.2022 11:08, Roger Pau Monne wrote: > >>> Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer > >>> being filled halfway during dom0 boot, and thus a non-trivial chunk of > >>> Linux boot messages are dropped. > >>> > >>> Increasing the buffer to 32K does fix the issue and Linux boot > >>> messages are no longer dropped. There's no justification either on > >>> why 16K was chosen, and hence bumping to 32K in order to cope with > >>> current systems generating output faster does seem appropriate to have > >>> a better user experience with the provided defaults. > >> > >> Just to record what was part of an earlier discussion: I'm not going > >> to nak such a change, but I think the justification is insufficient: > >> On this same basis someone else could come a few days later and bump > >> to 64k, then 128k, etc. > > > > Indeed, and that would be fine IMO. We should aim to provide defaults > > that work fine for most situations, and here I don't see what drawback > > it has to increase the default buffer size from 16kiB to 32kiB, and > > I would be fine with increasing to 128kiB if that's required for some > > use case, albeit I have a hard time seeing how we could fill that > > buffer. > > > > If I can ask, what kind of justification you would see fit for > > granting an increase to the default buffer size? > > Making plausible that for a majority of contemporary systems the buffer > is not large enough would be one aspect. But then there's imo always > going to be an issue: What if non-Linux Dom0 would be far more chatty? > What if Linux, down the road, was made less verbose (by default)? What > if people expect large enough a buffer to also suffice when Linux runs > in e.g. ignore_loglevel mode? We simply can't fit all use cases and at > the same time also not go overboard with the default size. That's why > there's a way to control this via command line option. Maybe I should clarify that the current buffer size is not enough on this system with the default Linux log level. I think we can expect someone that changes the default Linux log level to also consider changing the Xen buffer size. OTOH when using the default Linux log level the default Xen serial buffer should be enough. I haven't tested with FreeBSD on that system, other systems I have seem to be fine when booting FreeBSD with the default Xen serial buffer size. I think we should exercise caution if someone proposed to increase to 1M for example, but I don't see why so much controversy for going from 16K to 32K, it's IMO a reasonable value and has proven to prevent serial log loss when using the default Linux log level. Thanks, Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-29 16:19 ` Roger Pau Monné @ 2022-06-29 16:30 ` Jan Beulich 2022-06-30 8:07 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2022-06-29 16:30 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On 29.06.2022 18:19, Roger Pau Monné wrote: > On Wed, Jun 29, 2022 at 06:03:34PM +0200, Jan Beulich wrote: >> On 29.06.2022 17:23, Roger Pau Monné wrote: >>> On Thu, Jun 23, 2022 at 03:32:30PM +0200, Jan Beulich wrote: >>>> On 23.06.2022 11:08, Roger Pau Monne wrote: >>>>> Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer >>>>> being filled halfway during dom0 boot, and thus a non-trivial chunk of >>>>> Linux boot messages are dropped. >>>>> >>>>> Increasing the buffer to 32K does fix the issue and Linux boot >>>>> messages are no longer dropped. There's no justification either on >>>>> why 16K was chosen, and hence bumping to 32K in order to cope with >>>>> current systems generating output faster does seem appropriate to have >>>>> a better user experience with the provided defaults. >>>> >>>> Just to record what was part of an earlier discussion: I'm not going >>>> to nak such a change, but I think the justification is insufficient: >>>> On this same basis someone else could come a few days later and bump >>>> to 64k, then 128k, etc. >>> >>> Indeed, and that would be fine IMO. We should aim to provide defaults >>> that work fine for most situations, and here I don't see what drawback >>> it has to increase the default buffer size from 16kiB to 32kiB, and >>> I would be fine with increasing to 128kiB if that's required for some >>> use case, albeit I have a hard time seeing how we could fill that >>> buffer. >>> >>> If I can ask, what kind of justification you would see fit for >>> granting an increase to the default buffer size? >> >> Making plausible that for a majority of contemporary systems the buffer >> is not large enough would be one aspect. But then there's imo always >> going to be an issue: What if non-Linux Dom0 would be far more chatty? >> What if Linux, down the road, was made less verbose (by default)? What >> if people expect large enough a buffer to also suffice when Linux runs >> in e.g. ignore_loglevel mode? We simply can't fit all use cases and at >> the same time also not go overboard with the default size. That's why >> there's a way to control this via command line option. > > Maybe I should clarify that the current buffer size is not enough on > this system with the default Linux log level. I think we can expect > someone that changes the default Linux log level to also consider > changing the Xen buffer size. OTOH when using the default Linux log > level the default Xen serial buffer should be enough. > > I haven't tested with FreeBSD on that system, other systems I have > seem to be fine when booting FreeBSD with the default Xen serial > buffer size. > > I think we should exercise caution if someone proposed to increase to > 1M for example, but I don't see why so much controversy for going from > 16K to 32K, it's IMO a reasonable value and has proven to prevent > serial log loss when using the default Linux log level. But see - that's exactly my point. Where do you draw the line between "we should accept" and "exercise caution"? Is it 256k? Or 512k? Certainly I'm also aware of the common "memory is cheap" attitude, but that doesn't make me like it. That's both because of having started with 64k total (or maybe even less; too long ago meanwhile), and because of my general observation that everything only ever (fast) growing makes many things slower than they would need to be. As to controversy - I did make clear before that I don't mean to nak the change. But given my views, you'll need to find someone else to ack it despite being aware of my opinion. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] console/serial: bump buffer from 16K to 32K 2022-06-29 16:30 ` Jan Beulich @ 2022-06-30 8:07 ` Roger Pau Monné 0 siblings, 0 replies; 10+ messages in thread From: Roger Pau Monné @ 2022-06-30 8:07 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On Wed, Jun 29, 2022 at 06:30:18PM +0200, Jan Beulich wrote: > On 29.06.2022 18:19, Roger Pau Monné wrote: > > On Wed, Jun 29, 2022 at 06:03:34PM +0200, Jan Beulich wrote: > >> On 29.06.2022 17:23, Roger Pau Monné wrote: > >>> On Thu, Jun 23, 2022 at 03:32:30PM +0200, Jan Beulich wrote: > >>>> On 23.06.2022 11:08, Roger Pau Monne wrote: > >>>>> Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer > >>>>> being filled halfway during dom0 boot, and thus a non-trivial chunk of > >>>>> Linux boot messages are dropped. > >>>>> > >>>>> Increasing the buffer to 32K does fix the issue and Linux boot > >>>>> messages are no longer dropped. There's no justification either on > >>>>> why 16K was chosen, and hence bumping to 32K in order to cope with > >>>>> current systems generating output faster does seem appropriate to have > >>>>> a better user experience with the provided defaults. > >>>> > >>>> Just to record what was part of an earlier discussion: I'm not going > >>>> to nak such a change, but I think the justification is insufficient: > >>>> On this same basis someone else could come a few days later and bump > >>>> to 64k, then 128k, etc. > >>> > >>> Indeed, and that would be fine IMO. We should aim to provide defaults > >>> that work fine for most situations, and here I don't see what drawback > >>> it has to increase the default buffer size from 16kiB to 32kiB, and > >>> I would be fine with increasing to 128kiB if that's required for some > >>> use case, albeit I have a hard time seeing how we could fill that > >>> buffer. > >>> > >>> If I can ask, what kind of justification you would see fit for > >>> granting an increase to the default buffer size? > >> > >> Making plausible that for a majority of contemporary systems the buffer > >> is not large enough would be one aspect. But then there's imo always > >> going to be an issue: What if non-Linux Dom0 would be far more chatty? > >> What if Linux, down the road, was made less verbose (by default)? What > >> if people expect large enough a buffer to also suffice when Linux runs > >> in e.g. ignore_loglevel mode? We simply can't fit all use cases and at > >> the same time also not go overboard with the default size. That's why > >> there's a way to control this via command line option. > > > > Maybe I should clarify that the current buffer size is not enough on > > this system with the default Linux log level. I think we can expect > > someone that changes the default Linux log level to also consider > > changing the Xen buffer size. OTOH when using the default Linux log > > level the default Xen serial buffer should be enough. > > > > I haven't tested with FreeBSD on that system, other systems I have > > seem to be fine when booting FreeBSD with the default Xen serial > > buffer size. > > > > I think we should exercise caution if someone proposed to increase to > > 1M for example, but I don't see why so much controversy for going from > > 16K to 32K, it's IMO a reasonable value and has proven to prevent > > serial log loss when using the default Linux log level. > > But see - that's exactly my point. Where do you draw the line between > "we should accept" and "exercise caution"? Is it 256k? Or 512k? Hard to tell, it would depend on the justification/use case for needed those buffer sizes. To be fair 16K seems equally random to me, I tried to backtrack to the commit it was introduced, but I haven't been able to find any justification. I think we can at least agree that making the buffer size configurable from Kconfig is a desirable move. Thanks, Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-06-30 8:08 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-23 9:08 [PATCH 0/2] console/serial: adjust default TX buffer size Roger Pau Monne 2022-06-23 9:08 ` [PATCH 1/2] console/serial: set the default transmit buffer size in Kconfig Roger Pau Monne 2022-06-23 13:30 ` Jan Beulich 2022-06-23 9:08 ` [PATCH 2/2] console/serial: bump buffer from 16K to 32K Roger Pau Monne 2022-06-23 13:32 ` Jan Beulich 2022-06-29 15:23 ` Roger Pau Monné 2022-06-29 16:03 ` Jan Beulich 2022-06-29 16:19 ` Roger Pau Monné 2022-06-29 16:30 ` Jan Beulich 2022-06-30 8:07 ` Roger Pau Monné
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.