Re: [PATCH] ARM: add a private asm/unaligned.h

[Ard Biesheuvel]

On 1 November 2017 at 18:11, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxx> wrote:
> On Wed, Nov 01, 2017 at 06:02:24PM +0000, Ard Biesheuvel wrote:
>> On 1 November 2017 at 18:00, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxx> wrote:
>> > On Wed, Nov 01, 2017 at 03:57:36PM +0000, Ard Biesheuvel wrote:
>> >> On 31 October 2017 at 13:22, Gregory CLEMENT
>> >> <gregory.clement@xxxxxxxxxxxxxxxxxx> wrote:
>> >> > Hi Ard,
>> >> >
>> >> >  On mar., oct. 31 2017, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>> >> >
>> >> >> On 31 October 2017 at 12:47, Russell King - ARM Linux
>> >> >> <linux@xxxxxxxxxxxxxxx> wrote:
>> >> >>> On Mon, Oct 30, 2017 at 04:38:17PM +0000, Russell King - ARM Linux wrote:
>> >> >>>> On Mon, Oct 30, 2017 at 05:24:34PM +0100, Gregory CLEMENT wrote:
>> >> >>>> > Hi Russell King,
>> >> >>>> >
>> >> >>>> > Here you will find all the objects included the vmlinux:
>> >> >>>> >
>> >> >>>> > http://free-electrons.com/~gregory/pub/compressed.tgz
>> >> >>>>
>> >> >>>> Thanks.  Unfortunately, nothing stands out, but I do see a difference
>> >> >>>> between the output of your linker from mine.
>> >> >>>>
>> >> >>>> Yours:
>> >> >>>>
>> >> >>>> Idx Name          Size      VMA       LMA       File off  Algn
>> >> >>>>   0 .text         00005ef8  00000000  00000000  00010000  2**5
>> >> >>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>> >> >>>>
>> >> >>>> Mine:
>> >> >>>>
>> >> >>>> Idx Name          Size      VMA       LMA       File off  Algn
>> >> >>>>   0 .text         00005f00  00000000  00000000  00010000  2**5
>> >> >>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>> >> >>>>
>> >> >>>> That has the effect of moving the addresses of the following
>> >> >>>> sections in your vmlinux down by 8 bytes.  However, I don't think
>> >> >>>> that's the cause of this - but it does hint at something being
>> >> >>>> different in binutils in the way sections are processed in the
>> >> >>>> linker.
>> >> >>>>
>> >> >>>> Please add to your linker script after the assignment of _edata:
>> >> >>>>
>> >> >>>>   .image_end (NOLOAD) : {
>> >> >>>>     _edata_foo = .;
>> >> >>>>   }
>> >> >>>>
>> >> >>>> relink the decompressor, and see what value _edata_foo ends up with
>> >> >>>> compared to _edata?  They should be the same, but I suspect using
>> >> >>>> your linker, they will be different.
>> >> >>>>
>> >> >>>> Also try adding
>> >> >>>>     BYTE(0);
>> >> >>>>
>> >> >>>> after the _edata_foo assignment as a separate test, and see whether
>> >> >>>> that makes any difference - with that you should end up with the
>> >> >>>> .image_end section in the output image.
>> >> >>>
>> >> >>> Gregory sent me has new url... for _both_ changes, which gives me:
>> >> >
>> >> > If needed I can provide this url.
>> >> >
>> >> >>>
>> >> >>> $ arm-linux-nm vmlinux |grep _edata
>> >> >>> 00491160 D _edata
>> >> >>> 00491160 D _edata_foo
>> >> >>>
>> >> >>> So there's no reason that ASSERT() should be failing!  However, as I
>> >> >>> don't have the intermediate step, I can't say whether the addition
>> >> >>> of the BYTE() affected it in some way - sorry, but I asked for _both_
>> >> >>> to be tested above because I wanted to speed up the process, and
>> >> >>> clearly that's backfired.
>> >> >>>
>> >> >>> Given how close we potentially are to 4.14, I don't think we're going
>> >> >>> to get to the bottom of this to make 4.14.  I'd want to get this
>> >> >>> sorted by Wednesday so linux-next (which is resuming this evening)
>> >> >>> can grab a copy of my tree with it in, and we have another day to
>> >> >>> sort out any remaining issues, but I'm basically out of time to do
>> >> >>> anything further with this as of now.
>> >> >>
>> >> >>> So, 4.14 will likely be released without any of this being fixed.
>> >> >>>
>> >> >>
>> >> >> IIUC, the current issue is limited to the ASSERT() itself, which is
>> >> >> there to prevent future regressions, while the other two patches deal
>> >> >> with severe and difficult to diagnose known issues.
>> >> >
>> >> > I confirm that whithout the last commit (adding the ASSERT()) in the
>> >> > fixes branch it worked well.
>> >> >
>> >> >>
>> >> >> So why can't we apply those two patches as fixes, and revisit the
>> >> >> patch that helps us prevent this from regressing in the future for
>> >> >> v4.15?
>> >> >
>> >> > I also agree with this.
>> >> >
>> >>
>> >> Russell,
>> >>
>> >> Please drop the EFI patch from your tree. I will forward it myself.
>> >
>> > Really, no, I'm not going to.  I've enough to do than chase around
>> > playing political games about who should send what patches.  You
>> > asked me to merge it, and I will merge it.
>> >
>>
>> Fine, then merge it. I am not the one who is playing games here, I
>> just want to get stuff fixed. I don't understand why you are dragging
>> your feet like this.
>
> You think I'm playing games?  I most certainly am not.  I'm not going
> to send it tonight because there's further fixes in the pipeline from
> Nicolas, and I don't have time to merge those tonight _and_ test them.
> And I certainly do not want to be sending multiple pull requests to
> Linus, because that annoys Linus and I've been flamed for doing that.
>
> I haven't had time to drop the ASSERT() patch from my tree either since
> Tuesday morning.
>
> I guess you have very little patience and you have no appreciation of
> the fact that when someone states that they want to get an issue sorted
> quickly because of lack of time, they actually really do mean that they
> have very little availability...
>
> I said on Monday that time was short, and that I had precious little
> available time.  That was because I was out on Monday evening.  I was
> out on Tuesday afternoon for a medical appointment.  I was out Tuesday
> evening.  I've been out Wednesday afternoon.  This is not "games", this
> is reality, this is health issues.
>
> Have some patience and give your fellow developers some breathing space.
>

Apologies if that sounded rude, but the first fix I proposed for
Gregory's issue was sent on September 8th, i.e., almost two months
ago.

> Remember, many people have been at ELC and have been unavailable for
> much of last week.  Those who weren't at ELC were available, willing
> and able to try and address these issues, but it was impossible because
> of everyone else being away.  Now they're back, it seems they're banging
> on about getting some action.  That's really unfair - _they're_ the ones
> who've held up progress for a week.
>