Re: [RFT 1/2] printk: make dynamic kernel ring buffer alignemnt explicit

[Stephen Warren]

On 06/18/2014 05:14 AM, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
> 
> 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?).

IIRC the issue was that __log_buf's type is char[] so without the
__aligned it could have any alignment at all, e.g. 1 or 2. However,
struct printk_log is stored in the buffer rather than just char*, and so
if __log_buf isn't aligned to the required alignment for that structure,
that can caused unaligned accesses to fields in the structure, which
isn't supported on ARM in at least some cases.

As such, I think the change to setup_log_buf() in this patch makes sense
(although I suppose in practice memblock_virt_alloc() probably has some
minimum internal alignment that dwards LOG_ALIGN, but that's an
implementation detail we shouldn't rely on).
--
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/