Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs

[Luis R. Rodriguez]

On Mon, Jun 23, 2014 at 05:45:33PM -0700, Andrew Morton wrote:
> On Tue, 24 Jun 2014 02:20:50 +0200 "Luis R. Rodriguez" <mcgrof@xxxxxxxx> wrote:
> 
> > On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote:
> > > On Wed, 18 Jun 2014 13:45:37 -0700 "Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx> wrote:
> > > 
> > > > ...
> > > >
> > > > If an increase is required the ring buffer is increased to
> > > > +	  the next power of 2 that can fit both the minimum kernel ring buffer
> > > > +	  (LOG_BUF_SHIFT) plus the additional worst case CPU contributions.
> > > >
> > > > ...
> > > >
> > > > +	log_buf_len_update(cpu_extra + __LOG_BUF_LEN);
> > > > +}
> > > 
> > > I'd have expected
> > > 
> > >   total_cpu_space = minimum-per-cpu-len * nr_possible_cpus;
> > >   log_buf_len = max(__LOG_BUF_LEN, total_cpu_space)
> > > 
> > > but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work
> > > out why.  
> > > .
> > 
> > Ah, because its cpu_extra, not total_cpu_space that is being
> > computed, the goal was to see how much extra junk on the
> > worst case a CPU might contribute. The __LOG_BUF_LEN is the
> > default size, so we combine both.
> 
> Well...  why?  Isn't it simpler and more direct to say "I want at least
> 32k per CPU"?

That's certainly another way to go about this, but the original motivation
was trying to figure out the additional *extra* junk a CPU might spewed out,
it set out with an assumption of a base start from the first CPU booting the
system and that first CPU using the default kernel ring buffer size. The
language in the patch describes the worst case extra CPU junk contributed,
rather than a specific full split of the kernel ring buffer as that's typically
how extra junk is spewered out to the ring bufer and the concern. In general
on idle each CPU only contributes about only 1 to max 2 lines. The focus then
is the worst case on contribution.

Another note --  since this option depends on SMP and !BASE_SMALL technically 
num_possible_cpus() won't ever return something smaller than or equal to 1
but because of the default values chosen the -1 on the compuation does affect
whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the
-1 means we require > 64 CPUs.

This all can be changed however we like but the language and explained logic
would just need to be changed.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/