From: "Luis R. Rodriguez" <mcg...@suse.com> 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 <and...@lunn.ch> Cc: Stephen Warren <swar...@wwwdotorg.org> Cc: Michal Hocko <mho...@suse.cz> Cc: Petr Mladek <pmla...@suse.cz> Cc: Andrew Morton <a...@linux-foundation.org> Cc: Joe Perches <j...@perches.com> Cc: Arun KS <arunks.li...@gmail.com> Cc: Kees Cook <keesc...@chromium.org> Cc: Davidlohr Bueso <davidl...@hp.com> Cc: Chris Metcalf <cmetc...@tilera.com> Cc: linux-kernel@vger.kernel.org Signed-off-by: Luis R. Rodriguez <mcg...@suse.com> --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/