Re: [Xen-devel] patches pending acks (or naks)

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Dec 12, 2014 at 09:45:17AM +0000, Jan Beulich wrote:
> >>> On 11.12.14 at 20:41, <konrad.wilk@xxxxxxxxxx> wrote:
> > On Thu, Dec 11, 2014 at 03:38:39PM +0000, Jan Beulich wrote:
> >> >>> On 11.12.14 at 16:18, <konrad.wilk@xxxxxxxxxx> wrote:
> >> > A proper fix would be to automatically adjust based on memmap and CPU 
> >> > but 
> >> > that could be too complex.
> >> 
> >> The problem isn't the complexity, but the chicken-and-egg problem
> >> as far as CPU count is concerned. The memory map size would be
> >> known early enough.
> > 
> > Let me explain my thought process:
> >  - There are existing knobs that can be used to change this (conring_size=X)
> >  - But we would like an awesome release where those knobs don't have to
> >    be used.
> >  - The patch you posted could be reworked to fully address memmap and CPU.
> 
> Not really, unless we separate parsing and printing of the ACPI
> tables. Again, the CPU count is known only after that parsing.

Right, the logic of increasing the buffer based on CPU count is an
excellent addition.
> 
> >  - I don't know what your time constraints are for said patch and you
> >    might not have the energy to rework it.
> >  - I don't want to put pressure on you or Daniel on having to write new
> >    patches - especially as folks are going on vacation soon.
> > 
> > Hence as a fallback I believe that Daniel's patch should go in and
> > Jan's patch can go in too. However if Jan (or somebody else) wants to
> > rework the v2 (or a new one) to address the memmap issue then that
> > patch would go in - instead of Daniel's or v2 of Jan patch.
> 
> Which memmap issue? You confirmed in your reply that you understand
> that the memmap gets printed late enough for the change in v2 to
> take effect. Plus those are info-level messages, and hence don't get

Correct. The count of memmap entries can be high even with an
small amount of CPUs. Meaning your patch would not modify the
size of the circular buffer in such case (and we would lose some
of the memmap entries being printed). Daniel's patch would
provide a cushion by expanding the default size, however ..

> printed at all by default. And if somebody alters the log levels, (s)he
> can surely be expected to also adjust the ring size. (The log level
> aspect is actually another argument against Daniel's patch.)

... your point about the need to use 'loglvl' points out that
Daniel's patch does not fix the all-generic case.

/me puts on the Xen 4.6 todo 'adjust log buffer based on memmap size'

And your patch is:
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel