Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1278153
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages |
| Date | 2015-11-26 12:20 +0100 |
| Message-ID | <qz2KK-4oQ-1@gated-at.bofh.it> (permalink) |
| References | <qxZvA-2lA-7@gated-at.bofh.it> <qxZvA-2lA-5@gated-at.bofh.it> <qyh97-5D3-7@gated-at.bofh.it> <qyjui-72V-17@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
* 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/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-23 14:40 +0100
Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages Ingo Molnar <mingo@kernel.org> - 2015-11-24 09:30 +0100
Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-24 12:00 +0100
Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages Ingo Molnar <mingo@kernel.org> - 2015-11-26 12:20 +0100
Re: [PATCH 1/6] x86/efi: PFN_ALIGN() _text and _end when calculating number of pages Matt Fleming <matt@codeblueprint.co.uk> - 2015-11-27 22:10 +0100
csiph-web