On Wed 2014-06-18 04:14:24, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <[email protected]> > > We have to consider alignment for the ring buffer both for the > default static size, and then also for when an dynamic allocation > is made when the log_buf_len=n kernel parameter is passed to set > the size specifically to a size larger than the default size set > by the architecture through CONFIG_LOG_BUF_SHIFT. > > The default static kernel ring buffer can be aligned properly if > architectures set CONFIG_LOG_BUF_SHIFT properly, we provide ranges > for the size though so even if CONFIG_LOG_BUF_SHIFT has a sensible > aligned value it can be reduced to a non aligned value. Commit > 6ebb017de9 by Andrew ensures the static buffer is always aligned > and the decision of alignment is done by the compiler by using > __alignof__(struct log) (curious what value caused the crash?). > > When log_buf_len=n is used we allocate the ring buffer dynamically. > Dynamic allocation varies, for the early allocation called > before setup_arch() memblock_virt_alloc() requests a page aligment > and for the default kernel allocation memblock_virt_alloc_nopanic() > requests no special alignment, which in turn ends up aligning the > allocation to SMP_CACHE_BYTES, which is L1 cache aligned. > > Since we already have the required alignment for the kernel ring > buffer though we can do better and request explicit alignment for > LOG_ALIGN. Do that and also put the power of 2 practice of > the ring buffer size into a helper which we'll use later. > > Cc: Andrew Lunn <[email protected]> > Cc: Stephen Warren <[email protected]> > Cc: Michal Hocko <[email protected]> > Cc: Petr Mladek <[email protected]> > Cc: Andrew Morton <[email protected]> > Cc: Joe Perches <[email protected]> > Cc: Arun KS <[email protected]> > Cc: Kees Cook <[email protected]> > Cc: Davidlohr Bueso <[email protected]> > Cc: Chris Metcalf <[email protected]> > Cc: [email protected] > Signed-off-by: Luis R. Rodriguez <[email protected]>
The change makes sense to me. Acked-by: Petr Mladek <[email protected]> Tested-by: Petr Mladek <[email protected]> > This is perhaps not required given that we stick to powers of 2 > and the min LOG_BUF_SHIFT is 12, if the min LOG_BUF_SHIFT is > aligned then I think any passed log_buf_len=n would be aligned > as well as we don't make log_buf_len=n take effect unless > its > than the default size, and we round to the produced size > to the next power of 2. If the min length produced by > LOG_BUF_SHIFT is aligned, multiples of 2 of this should be as > well I think. > > This might be perhaps safest thing to do though given we'll > add other alloc entries next. > > kernel/printk/printk.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index ea2d5f6..af164a7 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -828,15 +828,21 @@ void log_buf_kexec_setup(void) > /* requested log_buf_len from kernel cmdline */ > static unsigned long __initdata new_log_buf_len; > > -/* save requested log_buf_len since it's too early to process it */ > -static int __init log_buf_len_setup(char *str) > +/* we practice scaling the ring buffer by powers of 2 */ > +static void __init log_buf_len_update(unsigned size) > { > - unsigned size = memparse(str, &str); > - > if (size) > size = roundup_pow_of_two(size); > if (size > log_buf_len) > new_log_buf_len = size; > +} > + > +/* save requested log_buf_len since it's too early to process it */ > +static int __init log_buf_len_setup(char *str) > +{ > + unsigned size = memparse(str, &str); > + > + log_buf_len_update(size); > > return 0; > } > @@ -853,9 +859,10 @@ void __init setup_log_buf(int early) > > if (early) { > new_log_buf = > - memblock_virt_alloc(new_log_buf_len, PAGE_SIZE); > + memblock_virt_alloc(new_log_buf_len, LOG_ALIGN); > } else { > - new_log_buf = memblock_virt_alloc_nopanic(new_log_buf_len, 0); > + new_log_buf = memblock_virt_alloc_nopanic(new_log_buf_len, > + LOG_ALIGN); > } > > if (unlikely(!new_log_buf)) { > -- > 1.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
