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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.12.14 at 17:41, <konrad.wilk@xxxxxxxxxx> wrote:
> 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).

I'm not following - the patch doesn't make the ring size dependent
on CPU count, that was the case already before. It only allows the
permanent ring buffer to be allocated quite a bit earlier thus
avoiding messages from getting overwritten.

> 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'

I'm not convinced this is a good idea, or else we should account for
possible other high volume sources too. Plus you'd still not be in the
position to auto-size the ring buffer in a way accounting for both
CPU count and memmap size.

Jan


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