[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/console: do not drop serial output from the hardware domain


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 22 Jun 2022 11:09:02 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=W2OTwPwlYSOyxYUZMM3ou862fvH/R2uq1iZS7krJxi8=; b=X1wCTiG56JvzQyCe06ZKfV9nZeFzBORJLsXRtbKO2bnSBlYY1+ZaJaNp7tVQz75dYy8jIhyEPSRqtnh/TouT74uynqxtsTH4bjrblUprSkTCwzgLr+v0lp8D/RE605nbbQrqiaw4Sq/sTSlj1cTBOKO5Sy/X3DlcVZAJ0/mbrMCAsIq3vFPjl2johahSnfGVNYYlLV5ft7UtrrPgJHxocmrG/bETEp2C8DS9V+j0gAq2oqxQvrilou3/oqtVFZpfxYVVzVp2q0epK5vl26JIYF6KtZjbXn1hT9HThD8txufQatS9gEjXS/t55ney4WmBFxpOKqxuyMDj1KVqez/FdQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=irRaTbhpNJH54YN3MzO+6tOHEwcwpanTs0OnBeH8joSDsHVvSvdjqkBB+ZwTZEVFQKoVInM5DTFSZkL4YJrLnSXmlfdb16wsnP9ARm6OzaiY+rQdIIEdrj/QQF8g770iTp/5gEoB9Q0N9+nrAZHSul9pEDJtRdn4I+33VqxMNypnjC3ASYXdAJ3bXr9BQCnte3K+zLcf76fiQURqKh3hU9+6wbAWIzVQ5gzqVrsp/HUHchY77YlpLJQtVXd85YQlGVWotYOppHcq90wPNW+jUZ8MADaVsvJ6tTc5zf6g/AdKqLVtz6Y8EF5ZAFnXHpCyu3JSixpWnXbQN4EGJGlGgQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 22 Jun 2022 09:09:31 +0000
  • Ironport-data: A9a23:ctQGya37X+qe0J4ksPbD5c9wkn2cJEfYwER7XKvMYLTBsI5bpzdUz 2YaXm2Cb67fM2Pwet5yYNyw9BtSuMLVydA1GwNqpC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkjk7xdOCn9xGQ7InQLlbGILes1htZGEk1Ek/NtTo5w7Rj2tAy24Dja++wk YiaT/P3aQfNNwFcagr424rbwP+4lK2v0N+wlgVWicFj5DcypVFMZH4sDfjZw0/DaptVBoaHq 9Prl9lVyI97EyAFUbtJmp6jGqEDryW70QKm0hK6UID66vROS7BbPg/W+5PwZG8O4whlkeydx /1vkZz3YiYnAZbdhesyWRlmNzxHD/VvreqvzXiX6aR/zmXgWl61m7BCKR9zOocVvOFqHWtJ6 PoUbigXaQyOjP63x7T9TfRwgsMkL4/gO4Z3VnNIlGmFS6p5B82TBfySuLe03x9p7ixKNezZa McDLyJmcTzLYgFVO0dRA5U79AutriakImwC9ALIzUYxy1GDwSVjyfv0CsOPc4bXX+lXsV2eg m2TqgwVBTlfbrRz0wGt8Hihm+vOliPTQ58JGfuz8fsCqF+Owm0eDjUGWF39puO24malQM5WI UEQ/isorIAx+VatQ927WAe3yFabujYMVtwWFPc1gCmdx6yR7wuHC2wsSj9adMdgpMIwXSYt1 FKCg5XuHzMHjVGOYXeU97PRoTbsPyEQdDcGfXVdFVZD5MT/qoYuiB6JVsxkDKO+ktzyH3f33 iyOqy89wb4UiKbnypmGwLwOuBr0zrChc+L/zl+/sr6Nhu+hWLOYWg==
  • Ironport-hdrordr: A9a23:iny006xSsfOFQxXvzoLKKrPxvuskLtp133Aq2lEZdPULSKGlfp GV9sjziyWetN9wYh4dcB67Scy9qFfnhOZICO4qTMyftWjdyRKVxeRZgbcKrAeBJ8STzJ8/6U 4kSdkFNDSSNykEsS+Z2njeLz9I+rDunsGVbKXlvhFQpGlRGt1dBmxCe2Km+yNNNWt77c1TLu vg2iMLnUvoRV0nKuCAQlUVVenKoNPG0LrgfB49HhYirC2Dlymh5rLWGwWRmk52aUIF/Z4StU z+1yDp7KSqtP+2jjfaym/o9pxT3P/s0MFKCsCggtUcbh/slgGrToJ8XKDqhkF8nMifrHIR1P XcqRYpOMp+r1vXY2GOuBPonzLt1T4/gkWSvWOwsD/Gm4jUVTg6A81OicZyaR3C8Xctu9l6ze Ziw3+Zn4A/N2KOoA3No/zzEz16nEu9pnQv1cQJiWZEbIcYYLhN6aQC4UJuFosaFi6S0vFqLA BXNrCc2B9qSyLbU5iA1VMfg+BEH05DUytue3Jy9PB8iFNt7TJEJ0hx/r1rop5PzuN5d3B+3Z W0Dk1ZrsAxciYoV9MMOA4ge7rBNoWfe2O7DIqtSW6XZ50vCjbql6PdxokTyaWDRKEopaFC6q gpFmko/1IPRw==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Jun 22, 2022 at 10:04:19AM +0200, Jan Beulich wrote:
> On 16.06.2022 13:31, Roger Pau Monné wrote:
> > On Tue, Jun 14, 2022 at 11:45:54AM +0200, Jan Beulich wrote:
> >> On 14.06.2022 11:38, Roger Pau Monné wrote:
> >>> On Tue, Jun 14, 2022 at 11:13:07AM +0200, Jan Beulich wrote:
> >>>> On 14.06.2022 10:32, Roger Pau Monné wrote:
> >>>>> On Tue, Jun 14, 2022 at 10:10:03AM +0200, Jan Beulich wrote:
> >>>>>> On 14.06.2022 08:52, Roger Pau Monné wrote:
> >>>>>>> On Mon, Jun 13, 2022 at 03:56:54PM +0200, Jan Beulich wrote:
> >>>>>>>> On 13.06.2022 14:32, Roger Pau Monné wrote:
> >>>>>>>>> On Mon, Jun 13, 2022 at 11:18:49AM +0200, Jan Beulich wrote:
> >>>>>>>>>> On 13.06.2022 11:04, Roger Pau Monné wrote:
> >>>>>>>>>>> On Mon, Jun 13, 2022 at 10:29:43AM +0200, Jan Beulich wrote:
> >>>>>>>>>>>> On 13.06.2022 10:21, Roger Pau Monné wrote:
> >>>>>>>>>>>>> On Mon, Jun 13, 2022 at 09:30:06AM +0200, Jan Beulich wrote:
> >>>>>>>>>>>>>> On 10.06.2022 17:06, Roger Pau Monne wrote:
> >>>>>>>>>>>>>>> Prevent dropping console output from the hardware domain, 
> >>>>>>>>>>>>>>> since it's
> >>>>>>>>>>>>>>> likely important to have all the output if the boot fails 
> >>>>>>>>>>>>>>> without
> >>>>>>>>>>>>>>> having to resort to sync_console (which also affects the 
> >>>>>>>>>>>>>>> output from
> >>>>>>>>>>>>>>> other guests).
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Do so by pairing the console_serial_puts() with
> >>>>>>>>>>>>>>> serial_{start,end}_log_everything(), so that no output is 
> >>>>>>>>>>>>>>> dropped.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> While I can see the goal, why would Dom0 output be 
> >>>>>>>>>>>>>> (effectively) more
> >>>>>>>>>>>>>> important than Xen's own one (which isn't "forced")? And with 
> >>>>>>>>>>>>>> this
> >>>>>>>>>>>>>> aiming at boot output only, wouldn't you want to stop the 
> >>>>>>>>>>>>>> overriding
> >>>>>>>>>>>>>> once boot has completed (of which, if I'm not mistaken, we 
> >>>>>>>>>>>>>> don't
> >>>>>>>>>>>>>> really have any signal coming from Dom0)? And even during boot 
> >>>>>>>>>>>>>> I'm
> >>>>>>>>>>>>>> not convinced we'd want to let through everything, but perhaps 
> >>>>>>>>>>>>>> just
> >>>>>>>>>>>>>> Dom0's kernel messages?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I normally use sync_console on all the boxes I'm doing dev 
> >>>>>>>>>>>>> work, so
> >>>>>>>>>>>>> this request is something that come up internally.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Didn't realize Xen output wasn't forced, since we already have 
> >>>>>>>>>>>>> rate
> >>>>>>>>>>>>> limiting based on log levels I was assuming that non-ratelimited
> >>>>>>>>>>>>> messages wouldn't be dropped.  But yes, I agree that Xen 
> >>>>>>>>>>>>> (non-guest
> >>>>>>>>>>>>> triggered) output shouldn't be rate limited either.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Which would raise the question of why we have log levels for 
> >>>>>>>>>>>> non-guest
> >>>>>>>>>>>> messages.
> >>>>>>>>>>>
> >>>>>>>>>>> Hm, maybe I'm confused, but I don't see a direct relation between 
> >>>>>>>>>>> log
> >>>>>>>>>>> levels and rate limiting.  If I set log level to WARNING I would
> >>>>>>>>>>> expect to not loose _any_ non-guest log messages with level 
> >>>>>>>>>>> WARNING or
> >>>>>>>>>>> above.  It's still useful to have log levels for non-guest 
> >>>>>>>>>>> messages,
> >>>>>>>>>>> since user might want to filter out DEBUG non-guest messages for
> >>>>>>>>>>> example.
> >>>>>>>>>>
> >>>>>>>>>> It was me who was confused, because of the two log-everything 
> >>>>>>>>>> variants
> >>>>>>>>>> we have (console and serial). You're right that your change is 
> >>>>>>>>>> unrelated
> >>>>>>>>>> to log levels. However, when there are e.g. many warnings or when 
> >>>>>>>>>> an
> >>>>>>>>>> admin has lowered the log level, what you (would) do is effectively
> >>>>>>>>>> force sync_console mode transiently (for a subset of messages, but
> >>>>>>>>>> that's secondary, especially because the "forced" output would 
> >>>>>>>>>> still
> >>>>>>>>>> be waiting for earlier output to make it out).
> >>>>>>>>>
> >>>>>>>>> Right, it would have to wait for any previous output on the buffer 
> >>>>>>>>> to
> >>>>>>>>> go out first.  In any case we can guarantee that no more output will
> >>>>>>>>> be added to the buffer while Xen waits for it to be flushed.
> >>>>>>>>>
> >>>>>>>>> So for the hardware domain it might make sense to wait for the TX
> >>>>>>>>> buffers to be half empty (the current tx_quench logic) by preempting
> >>>>>>>>> the hypercall.  That however could cause issues if guests manage to
> >>>>>>>>> keep filling the buffer while the hardware domain is being 
> >>>>>>>>> preempted.
> >>>>>>>>>
> >>>>>>>>> Alternatively we could always reserve half of the buffer for the
> >>>>>>>>> hardware domain, and allow it to be preempted while waiting for 
> >>>>>>>>> space
> >>>>>>>>> (since it's guaranteed non hardware domains won't be able to steal 
> >>>>>>>>> the
> >>>>>>>>> allocation from the hardware domain).
> >>>>>>>>
> >>>>>>>> Getting complicated it seems. I have to admit that I wonder whether 
> >>>>>>>> we
> >>>>>>>> wouldn't be better off leaving the current logic as is.
> >>>>>>>
> >>>>>>> Another possible solution (more like a band aid) is to increase the
> >>>>>>> buffer size from 4 pages to 8 or 16.  That would likely allow to cope
> >>>>>>> fine with the high throughput of boot messages.
> >>>>>>
> >>>>>> You mean the buffer whose size is controlled by serial_tx_buffer?
> >>>>>
> >>>>> Yes.
> >>>>>
> >>>>>> On
> >>>>>> large systems one may want to simply make use of the command line
> >>>>>> option then; I don't think the built-in default needs changing. Or
> >>>>>> if so, then perhaps not statically at build time, but taking into
> >>>>>> account system properties (like CPU count).
> >>>>>
> >>>>> So how about we use:
> >>>>>
> >>>>> min(16384, ROUNDUP(1024 * num_possible_cpus(), 4096))
> >>>>
> >>>> That would _reduce_ size on small systems, wouldn't it? Originally
> >>>> you were after increasing the default size. But if you had meant
> >>>> max(), then I'd fear on very large systems this may grow a little
> >>>> too large.
> >>>
> >>> See previous followup about my mistake of using min() instead of
> >>> max().
> >>>
> >>> On a system with 512 CPUs that would be 512KB, I don't think that's a
> >>> lot of memory, specially taking into account that a system with 512
> >>> CPUs should have a matching amount of memory I would expect.
> >>>
> >>> It's true however that I very much doubt we would fill a 512K buffer,
> >>> so limiting to 64K might be a sensible starting point?
> >>
> >> Yeah, 64k could be a value to compromise on. What total size of
> >> output have you observed to trigger the making of this patch? Xen
> >> alone doesn't even manage to fill 16k on most of my systems ...
> > 
> > I've tried on one of the affected systems now, it's a 8 CPU Kaby Lake
> > at 3,5GHz, and manages to fill the buffer while booting Linux.
> > 
> > My proposed formula won't fix this use case, so what about just
> > bumping the buffer to 32K by default, which does fix it?
> 
> As said, suitably explained I could also agree with going to 64k. The
> question though is in how far 32k, 64k, or ...
> 
> > Or alternatively use the proposed formula, but adjust the buffer to be
> > between [32K,64K].
> 
> ... this formula would cover a wide range of contemporary systems.
> Without such I can't really see what good a bump would do, as then
> many people may still find themselves in need of using the command
> line option to put in place a larger buffer.

I'm afraid I don't know how to make progress with this.

The current value is clearly too low for at least one of my systems.
I don't think it's feasible for me to propose a value or formula that
I can confirm will be suitable for all systems, hence I would suggest
increasing the buffer value to 32K as that does fix the problem on
that specific system (without claiming it's a value that would suit
all setups).

I agree that many people could still find themselves in the need of
using the command line option, but I can assure that new buffer value
would fix the issue on at least one system, which should be enough as
a justification.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.