Patchwork [GIT,PULL,v2] Linux support for ARM LPAE

login
register
mail settings
Submitter Catalin Marinas
Date Dec. 6, 2011, 3:29 p.m.
Message ID <20111206152955.GC31720@arm.com>
Download mbox
Permalink /patch/129710/
State New
Headers show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/cmarinas/linux.git for-next

Comments

Catalin Marinas - Dec. 6, 2011, 3:29 p.m.
Hi Russell,

I updated the series with an additional signed-off-by to your patch. The
code is unchanged. Could you please pull it again?

Thanks.

The following changes since commit 5611cc4572e889b62a7b4c72a413536bf6a9c416:

  Linux 3.2-rc4 (2011-12-01 14:56:01 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/cmarinas/linux.git for-next

Catalin Marinas (12):
      ARM: pgtable: Fix compiler warning in ioremap.c introduced by nopud
      ARM: LPAE: Move page table maintenance macros to pgtable-2level.h
      ARM: LPAE: Move the FSR definitions to separate files
      ARM: LPAE: Factor out classic-MMU specific code into proc-v7-2level.S
      ARM: LPAE: Introduce the 3-level page table format definitions
      ARM: LPAE: Page table maintenance for the 3-level format
      ARM: LPAE: MMU setup for the 3-level page table format
      ARM: LPAE: Invalidate the TLB before freeing the PMD
      ARM: LPAE: Add fault handling support
      ARM: LPAE: Add context switching support
      ARM: LPAE: Add identity mapping support for the 3-level page table format
      ARM: LPAE: Add the Kconfig entries

Russell King (1):
      ARM: pgtable: switch to use pgtable-nopud.h

Will Deacon (2):
      ARM: LPAE: add ISBs around MMU enabling code
      ARM: LPAE: mark memory banks with start > ULONG_MAX as highmem

 arch/arm/Kconfig                            |    2 +-
 arch/arm/boot/compressed/head.S             |    1 +
 arch/arm/include/asm/assembler.h            |   11 ++
 arch/arm/include/asm/page.h                 |    4 +
 arch/arm/include/asm/pgalloc.h              |   26 ++++-
 arch/arm/include/asm/pgtable-2level.h       |   41 ++++++
 arch/arm/include/asm/pgtable-3level-hwdef.h |   77 ++++++++++++
 arch/arm/include/asm/pgtable-3level-types.h |   70 +++++++++++
 arch/arm/include/asm/pgtable-3level.h       |  155 +++++++++++++++++++++++
 arch/arm/include/asm/pgtable-hwdef.h        |    4 +
 arch/arm/include/asm/pgtable.h              |   43 +------
 arch/arm/include/asm/proc-fns.h             |   21 +++
 arch/arm/include/asm/system.h               |    8 ++
 arch/arm/include/asm/tlb.h                  |   12 ++-
 arch/arm/kernel/head.S                      |   47 +++++++-
 arch/arm/kernel/hw_breakpoint.c             |    8 +-
 arch/arm/kernel/sleep.S                     |    2 +
 arch/arm/mm/Kconfig                         |   17 +++
 arch/arm/mm/alignment.c                     |    2 +-
 arch/arm/mm/context.c                       |   19 +++-
 arch/arm/mm/fault.c                         |  111 +++--------------
 arch/arm/mm/fault.h                         |   27 ++++-
 arch/arm/mm/fsr-2level.c                    |   78 ++++++++++++
 arch/arm/mm/fsr-3level.c                    |   68 ++++++++++
 arch/arm/mm/idmap.c                         |   34 +++++-
 arch/arm/mm/ioremap.c                       |   39 ++++---
 arch/arm/mm/mmu.c                           |   46 +++++++-
 arch/arm/mm/pgd.c                           |   51 +++++++-
 arch/arm/mm/proc-macros.S                   |    5 +-
 arch/arm/mm/proc-v7-2level.S                |  171 ++++++++++++++++++++++++++
 arch/arm/mm/proc-v7-3level.S                |  150 +++++++++++++++++++++++
 arch/arm/mm/proc-v7.S                       |  177 +++------------------------
 32 files changed, 1204 insertions(+), 323 deletions(-)
 create mode 100644 arch/arm/include/asm/pgtable-3level-hwdef.h
 create mode 100644 arch/arm/include/asm/pgtable-3level-types.h
 create mode 100644 arch/arm/include/asm/pgtable-3level.h
 create mode 100644 arch/arm/mm/fsr-2level.c
 create mode 100644 arch/arm/mm/fsr-3level.c
 create mode 100644 arch/arm/mm/proc-v7-2level.S
 create mode 100644 arch/arm/mm/proc-v7-3level.S
Russell King - ARM Linux - Dec. 6, 2011, 8:34 p.m.
On Tue, Dec 06, 2011 at 03:29:55PM +0000, Catalin Marinas wrote:
> I updated the series with an additional signed-off-by to your patch. The
> code is unchanged. Could you please pull it again?

I will for this time only, but note - last time you said:

> If there are no further comments, could you please pull the ARM LPAE
> branch below? They should be merged after Will's idmap patches (no real
> conflict, just correctly functioning setup_mm_for_reboot).

Now, the thing is, merging it after Will's patches won't solve that in
any shape or form - it really does not matter what order I do the merges
in, the fact of the matter is that there is no ordering or dependence
between your patches and Will's, so there's no guarantee that your code
will see a properly functioning setup_mm_for_reboot.

If you actually depend on something in some other tree, you need to have
it merged into your tree _before_ the objects which depend on it.

So, what the above comment admits to me is that "what I'm asking you to
pull is known to be broken if a git-bisect hits one of these commits,
but I don't care."

As I said, I will merge it this time around, but next time I won't.
Russell King - ARM Linux - Dec. 6, 2011, 8:45 p.m.
On Tue, Dec 06, 2011 at 08:34:16PM +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 06, 2011 at 03:29:55PM +0000, Catalin Marinas wrote:
> > I updated the series with an additional signed-off-by to your patch. The
> > code is unchanged. Could you please pull it again?
> 
> I will for this time only, but note - last time you said:
> 
> > If there are no further comments, could you please pull the ARM LPAE
> > branch below? They should be merged after Will's idmap patches (no real
> > conflict, just correctly functioning setup_mm_for_reboot).
> 
> Now, the thing is, merging it after Will's patches won't solve that in
> any shape or form - it really does not matter what order I do the merges
> in, the fact of the matter is that there is no ordering or dependence
> between your patches and Will's, so there's no guarantee that your code
> will see a properly functioning setup_mm_for_reboot.
> 
> If you actually depend on something in some other tree, you need to have
> it merged into your tree _before_ the objects which depend on it.
> 
> So, what the above comment admits to me is that "what I'm asking you to
> pull is known to be broken if a git-bisect hits one of these commits,
> but I don't care."
> 
> As I said, I will merge it this time around, but next time I won't.

Well, I might do if you told me that your changes conflict with Will's
idmap changes in arch/arm/mm/idmap.c:

++<<<<<<< HEAD
 +extern char  __idmap_text_start[], __idmap_text_end[];
++=======
+ #ifdef CONFIG_SMP
+ static void idmap_del_pmd(pud_t *pud, unsigned long addr, unsigned long end)
+ {
+       pmd_t *pmd;
+
+       if (pud_none_or_clear_bad(pud))
+               return;
+       pmd = pmd_offset(pud, addr);
+       pmd_clear(pmd);
+ }
++>>>>>>> cf9a53fb7cf8dd2fd2b5d7bb07434bd2ebeb5a8f

I'm assuming that idmap_del_pmd() just gets deleted?

Alternatively, if you wish to fix the dependencies of your commits and
resubmit a pull request, let me know.
Catalin Marinas - Dec. 6, 2011, 10:24 p.m.
On 6 December 2011 20:34, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Dec 06, 2011 at 03:29:55PM +0000, Catalin Marinas wrote:
>> I updated the series with an additional signed-off-by to your patch. The
>> code is unchanged. Could you please pull it again?
>
> I will for this time only, but note - last time you said:
>
>> If there are no further comments, could you please pull the ARM LPAE
>> branch below? They should be merged after Will's idmap patches (no real
>> conflict, just correctly functioning setup_mm_for_reboot).
>
> Now, the thing is, merging it after Will's patches won't solve that in
> any shape or form - it really does not matter what order I do the merges
> in, the fact of the matter is that there is no ordering or dependence
> between your patches and Will's, so there's no guarantee that your code
> will see a properly functioning setup_mm_for_reboot.

This situation is not unique to my LPAE patches (git bisect doesn't
always go well merges). There are many other situations where one
relies on some stuff to get into mainline (or maintainer tree) before
merging. Just have a look at the kernel logs.

But if you want, there other solutions:

1. I can rebase the LPAE branch on top of your devel-stable _after_
you merged Will's patches. Then I send another pull request.

2. Only merge the LPAE patches without the one adding the Kconfig
option. Later apply the Kconfig patch.

> If you actually depend on something in some other tree, you need to have
> it merged into your tree _before_ the objects which depend on it.

Not easy in this instance. AFAIK Will's code relied on some reset
patches from you, so Will just used your devel-stable branch.

> As I said, I will merge it this time around, but next time I won't.

As I also said above, there are solutions. What it is really needed is
_better_ collaboration during merges and _discussing_ the best
approach rather than threatening that there won't be other pulls. I'm
open to rebase my branch against other commits, just say it. I doubt
this is the only time where merge issues occured.
Catalin Marinas - Dec. 6, 2011, 10:27 p.m.
On 6 December 2011 20:45, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Dec 06, 2011 at 08:34:16PM +0000, Russell King - ARM Linux wrote:
>> On Tue, Dec 06, 2011 at 03:29:55PM +0000, Catalin Marinas wrote:
>> > I updated the series with an additional signed-off-by to your patch. The
>> > code is unchanged. Could you please pull it again?
>>
>> I will for this time only, but note - last time you said:
>>
>> > If there are no further comments, could you please pull the ARM LPAE
>> > branch below? They should be merged after Will's idmap patches (no real
>> > conflict, just correctly functioning setup_mm_for_reboot).
>>
>> Now, the thing is, merging it after Will's patches won't solve that in
>> any shape or form - it really does not matter what order I do the merges
>> in, the fact of the matter is that there is no ordering or dependence
>> between your patches and Will's, so there's no guarantee that your code
>> will see a properly functioning setup_mm_for_reboot.
>>
>> If you actually depend on something in some other tree, you need to have
>> it merged into your tree _before_ the objects which depend on it.
>>
>> So, what the above comment admits to me is that "what I'm asking you to
>> pull is known to be broken if a git-bisect hits one of these commits,
>> but I don't care."
>>
>> As I said, I will merge it this time around, but next time I won't.
>
> Well, I might do if you told me that your changes conflict with Will's
> idmap changes in arch/arm/mm/idmap.c:
>
> ++<<<<<<< HEAD
>  +extern char  __idmap_text_start[], __idmap_text_end[];
> ++=======
> + #ifdef CONFIG_SMP
> + static void idmap_del_pmd(pud_t *pud, unsigned long addr, unsigned long end)
> + {
> +       pmd_t *pmd;
> +
> +       if (pud_none_or_clear_bad(pud))
> +               return;
> +       pmd = pmd_offset(pud, addr);
> +       pmd_clear(pmd);
> + }
> ++>>>>>>> cf9a53fb7cf8dd2fd2b5d7bb07434bd2ebeb5a8f
>
> I'm assuming that idmap_del_pmd() just gets deleted?
>
> Alternatively, if you wish to fix the dependencies of your commits and
> resubmit a pull request, let me know.

Indeed, idmap_del_pmd() is deleted.

But a better approach if you already merged Will's idmap branch is for
me to rebase LPAE against your devel-stable. I'll solve the conflicts
and test the branch before sending another pull request. If you prefer
this, is there some commit I can rebase LPAE onto?
Russell King - ARM Linux - Dec. 6, 2011, 11:16 p.m.
On Tue, Dec 06, 2011 at 10:24:28PM +0000, Catalin Marinas wrote:
> On 6 December 2011 20:34, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Dec 06, 2011 at 03:29:55PM +0000, Catalin Marinas wrote:
> >> I updated the series with an additional signed-off-by to your patch. The
> >> code is unchanged. Could you please pull it again?
> >
> > I will for this time only, but note - last time you said:
> >
> >> If there are no further comments, could you please pull the ARM LPAE
> >> branch below? They should be merged after Will's idmap patches (no real
> >> conflict, just correctly functioning setup_mm_for_reboot).
> >
> > Now, the thing is, merging it after Will's patches won't solve that in
> > any shape or form - it really does not matter what order I do the merges
> > in, the fact of the matter is that there is no ordering or dependence
> > between your patches and Will's, so there's no guarantee that your code
> > will see a properly functioning setup_mm_for_reboot.
> 
> This situation is not unique to my LPAE patches (git bisect doesn't
> always go well merges).

We should _strive_ to make git bisect work where possible.  It's merge
windows where the most breakage happens, and if they're not bisectable
then that kills a valuable debugging tool.  With more and more thousands
of commits going in to each merge window, it's becoming increasingly
important that it works.

> But if you want, there other solutions:
> 
> 1. I can rebase the LPAE branch on top of your devel-stable _after_
> you merged Will's patches. Then I send another pull request.
> 
> 2. Only merge the LPAE patches without the one adding the Kconfig
> option. Later apply the Kconfig patch.
> 
> > If you actually depend on something in some other tree, you need to have
> > it merged into your tree _before_ the objects which depend on it.
> 
> Not easy in this instance. AFAIK Will's code relied on some reset
> patches from you, so Will just used your devel-stable branch.

I'll grant you that it hasn't been easy, because Will has had to rebase
his git tree at my request, because of the breakage of Samsung and OMAP
with Marc's irqchip-consolidation patches.

Nevertheless, we're at -rc3, which means there is no reason to rush this.
There is no problem with you waiting until Will's idmap patches have been
merged (they have) and then moving on from there.

> > As I said, I will merge it this time around, but next time I won't.
> 
> As I also said above, there are solutions. What it is really needed is
> _better_ collaboration during merges and _discussing_ the best
> approach rather than threatening that there won't be other pulls.

And what about the email from Philippe on me rejecting your first pull
request?  There is no need to try to exert commercial pressures when the
problems are legal and technical.
Catalin Marinas - Dec. 7, 2011, 10:59 a.m.
On Tue, Dec 06, 2011 at 11:16:30PM +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 06, 2011 at 10:24:28PM +0000, Catalin Marinas wrote:
> > On 6 December 2011 20:34, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> > > As I said, I will merge it this time around, but next time I won't.
> > 
> > As I also said above, there are solutions. What it is really needed is
> > _better_ collaboration during merges and _discussing_ the best
> > approach rather than threatening that there won't be other pulls.
> 
> And what about the email from Philippe on me rejecting your first pull
> request?  There is no need to try to exert commercial pressures when the
> problems are legal and technical.

I'm not discussing Philippe's email publicly (as it was in private).

>From my perspective, the tone of your email suggested more of a good
opportunity not to merge LPAE (and in subsequent emails doing ARM a
great favour by only accepting it this time) rather than a more
constructive 'fix this before merging'.

Since you mentioned commercial pressures, yes, there are many, but I'm
not sure you are aware of all of them (many companies haven't publicly
announced their plans, though they have very aggressive targets). I am
asked on a weekly basis when features like LPAE (or other bug-fixes -
TLBs, ASIDs) get into mainline. I can't really answer as it does not
depend on me and I don't have any visibility on when it would happen.
Long delays is not a problem, it's predictability that matters. As a
temporary (can be 1-2 years) workaround, I have to recommend them to use
one of my development trees (e.g. linux-arm-arch).

On the positive side, more and more companies rely on the mainline
kernel and many of them don't even trust a set of patches unless it's in
the mainline kernel. The Linux community complained in the past that
companies forked the kernel and used old versions but now we should
really help those that want to stay up to date with the latest mainline.
Russell King - ARM Linux - Dec. 7, 2011, 8:01 p.m.
On Wed, Dec 07, 2011 at 10:59:13AM +0000, Catalin Marinas wrote:
> On Tue, Dec 06, 2011 at 11:16:30PM +0000, Russell King - ARM Linux wrote:
> > On Tue, Dec 06, 2011 at 10:24:28PM +0000, Catalin Marinas wrote:
> > > On 6 December 2011 20:34, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > > > As I said, I will merge it this time around, but next time I won't.
> > > 
> > > As I also said above, there are solutions. What it is really needed is
> > > _better_ collaboration during merges and _discussing_ the best
> > > approach rather than threatening that there won't be other pulls.
> > 
> > And what about the email from Philippe on me rejecting your first pull
> > request?  There is no need to try to exert commercial pressures when the
> > problems are legal and technical.
> 
> I'm not discussing Philippe's email publicly (as it was in private).
> 
> >From my perspective, the tone of your email suggested more of a good
> opportunity not to merge LPAE (and in subsequent emails doing ARM a
> great favour by only accepting it this time) rather than a more
> constructive 'fix this before merging'.

For fuck sake Catalin - which bit of "I MERGED IT AND THEN I HAD TO
THROW IT OUT" do you not understand?

I said precisely what needed to be said.  You fucked up because you
didn't follow the DCO.  Plain and simple.  And I had to throw it out
of an already published devel-stable branch which had been published
for 9+ hours.  Of course I'd be annoyed by that - that mistake means
I *HAD* to break the promise I made to the rest of the community that
I would never rewind the devel-stable branch, ever.

For someone who has been working on the kernel for a great number of
years, you show quite a lack of understanding of community process
when you slip up like that, and _then_ have the audacity to say that
it's a 'minor issue' (your words not mine.)

> Since you mentioned commercial pressures, yes, there are many, but I'm
> not sure you are aware of all of them (many companies haven't publicly
> announced their plans, though they have very aggressive targets). I am
> asked on a weekly basis when features like LPAE (or other bug-fixes -
> TLBs, ASIDs) get into mainline. I can't really answer as it does not
> depend on me and I don't have any visibility on when it would happen.
> Long delays is not a problem, it's predictability that matters. As a
> temporary (can be 1-2 years) workaround, I have to recommend them to use
> one of my development trees (e.g. linux-arm-arch).

None of that should _ever_ be used as any kind of motivation to get
something into mainline if it's not ready to be there.  Clearly, from
the issues raised with your tree thus far it wasn't actually ready.

Ready does not mean "it builds for me".  Ready means it builds, it's
tested, it's got proper commit messages, attributations and sign-offs.
If it's not at that stage, then quite simply it is not ready.  There's
no two ways about that.

And as I've already explained to you several times now, since sign-offs
are seen by many to be a legal statement, incorrect sign-offs are not
a minor problem.  Had it been correct, I would not have had to throw it
out of my tree.

So please, get it through your head that _had_ the sign-offs been correct
we wouldn't be having this discussion and your tree wouldn't have been
thrown out - and it would've actually been merged _before_ Will's idmap
changes.