Date: Thu, 26 Nov 2015 12:13:23 +0100
From: Ingo Molnar <mingo@kernel.org>
To: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>, "H . Peter Anvin" <hpa@zytor.com>,
        Toshi Kani <toshi.kani@hp.com>, linux-kernel@vger.kernel.org,
        linux-efi@vger.kernel.org, Borislav Petkov <bp@alien8.de>,
        Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
        stable@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>
Subject: Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating
 number of pages
Message-ID: <20151126111323.GA22863@gmail.com>
References: <1448285594-17617-1-git-send-email-matt@codeblueprint.co.uk>
 <1448285594-17617-2-git-send-email-matt@codeblueprint.co.uk>
 <20151124082323.GA23451@gmail.com>
 <20151124105556.GA2460@codeblueprint.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20151124105556.GA2460@codeblueprint.co.uk>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1988
Lines: 52


* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Tue, 24 Nov, at 09:23:23AM, Ingo Molnar wrote:
> > 
> > Didn't we want to do the _end alignment linker script fix instead?
>  
> I think we should do both. This patch is tagged for stable because it
> fixes a bug in the existing code. It's obvious and it's explicit and
> it's much easier to know when someone might want to backport it.
> 
> Changing the linker script which indirectly fixes the above bug is a
> much more subtle solution, with much larger potential for fallout
> because it affects multiple chunks of kernel code.
> 
> > Alignment assumptions are easy to make when symbols are well aligned typically (as 
> > in this case), so we should guarantee the alignment property instead of 
> > complicating the code.
> 
> I don't agree that sprinkling PFN_ALIGN() complicates the code, it's a
> minimal change with a well known kernel idiom. But yes, aligning these
> symbols in the linker script is generally a good idea.
> 
> The two patches are worthwhile, for different reasons; let's do both.

I disagree, this form:

        npages = (_end - _text) >> PAGE_SHIFT;

is a lot clearer to read than:

        npages = (PFN_ALIGN(_end) - PFN_ALIGN(_text)) >> PAGE_SHIFT;

especially once we ensure that _end and _text are page aligned. The latter form 
will only result in cargo-cult carrying over of unnecessary PFN_ALIGN() 
operations.

Section boundaries of the kernel should generally be page aligned, this is useful 
for a number of other reasons as well.

As far as backporting goes, it would generally be _safer_ to backport the linker 
script fix, in case there are other unrealized alignment bugs in the kernel. 
Especially if upstream does the same.

Thanks,

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