All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
To: Scott Wood <scottwood@freescale.com>, Rex Feany <RFeany@mrv.com>
Cc: "linuxppc-dev@ozlabs.org" <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
Date: Sun, 4 Oct 2009 22:10:32 +0200	[thread overview]
Message-ID: <OF7C895670.A7F16A1A-ONC1257645.00620A4D-C1257645.006ED40F@transmode.se> (raw)
In-Reply-To: <20091002214949.GA20514@b07421-ec1.am.freescale.net>

Scott Wood <scottwood@freescale.com> wrote on 02/10/2009 23:49:49:
>
> On Thu, Oct 01, 2009 at 08:35:59AM +1000, Benjamin Herrenschmidt wrote:
> > >From what I can see, the TLB miss code will check _PAGE_PRESENT, and
> > when not set, it will -still- insert something into the TLB (unlike
> > all other CPU types that go straight to data access faults from there).
> >
> > So we end up populating with those unpopulated entries that will then
> > cause us to take a DSI (or ISI) instead of a TLB miss the next time
> > around ... and so we need to remove them once we do that, no ? IE. Once
> > we have actually put a valid PTE in.
> >
> > At least that's my understanding and why I believe we should always have
> > tlbil_va() in set_pte() and ptep_set_access_flags(), which boils down
> > in putting it into the 2 "filter" functions in the new code.
> >
> > Well.. actually, the ptep_set_access_flags() will already do a
> > flush_tlb_page_nohash() when the PTE is changed. So I suppose all we
> > really need here would be in set_pte_filter(), to do the same if we are
> > writing a PTE that has _PAGE_PRESENT, at least on 8xx.
> >
> > But just unconditionally doing a tlbil_va() in both the filter functions
> > shouldn't harm and also fix the problem, though Rex seems to indicate
> > that is not the case.
>
> Adding a tlbil_va to do_page_fault makes the problem go away for me (on
> top of your "merge" branch) -- none of the other changes in this thread
> do (assuming I didn't miss any).  FWIW, when it gets stuck on a fault,
> DSISR is 0xc0000000, and handle_mm_fault returns zero.

Scott and Rex

I have managed to update the TLB code to make proper use of dirty and accessed states.
Advantages are:
 - I/D TLB Miss never needs to write to the linux pte, saving a few cycles
 - Accessed is only set by I/D TLB Error, should be a plus when SWAP is used.
 - _PAGE_DIRTY is mapped to 0x100, the changed bit, and is set directly
    and there will be no extra DTLB Error to actually set the changed bit
    when a page has been made dirty.
- Proper RO/RW mapping of user space.

Cons:
 - 4 more insn in TLB Miss handlers, but the since the linux pte isn't
   written it should still be a win.

However, I did this on my 2.4 tree but I can port it to 2.6 if you guys
can test it for me.

 Jocke

  parent reply	other threads:[~2009-10-04 20:15 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-24  0:45 [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite Rex Feany
2009-09-24  6:44 ` Benjamin Herrenschmidt
2009-09-24 23:33   ` Rex Feany
2009-09-24 23:52     ` Benjamin Herrenschmidt
2009-09-25  1:35       ` Rex Feany
2009-09-25  1:51         ` Benjamin Herrenschmidt
2009-09-25  3:03           ` Benjamin Herrenschmidt
2009-09-25  8:31             ` Joakim Tjernlund
2009-09-25  9:47               ` Benjamin Herrenschmidt
2009-09-25 10:21                 ` Joakim Tjernlund
2009-09-25 21:18             ` Rex Feany
2009-09-27 13:22               ` Joakim Tjernlund
2009-09-28  3:21                 ` Benjamin Herrenschmidt
2009-09-28  7:22                   ` Joakim Tjernlund
2009-09-28  7:34                     ` Benjamin Herrenschmidt
2009-09-28  7:39                       ` Joakim Tjernlund
2009-09-28 10:02                     ` Joakim Tjernlund
2009-09-29  1:21           ` Rex Feany
2009-09-29  6:26             ` Joakim Tjernlund
2009-09-29  7:07               ` Benjamin Herrenschmidt
2009-09-29  8:13                 ` Joakim Tjernlund
2009-09-29  8:16                   ` Benjamin Herrenschmidt
2009-09-29  8:24                     ` Joakim Tjernlund
2009-09-29 11:56                     ` Joakim Tjernlund
2009-09-29 21:03                       ` Rex Feany
2009-09-30  7:59                         ` Joakim Tjernlund
2009-09-30  8:19                           ` Joakim Tjernlund
2009-09-30  9:00                             ` Rex Feany
2009-09-30  9:58                               ` Joakim Tjernlund
2009-09-30 11:18                               ` Joakim Tjernlund
2009-09-30 17:23                                 ` Joakim Tjernlund
2009-09-30 22:35                                   ` Benjamin Herrenschmidt
2009-10-01  7:05                                     ` Joakim Tjernlund
2009-10-02 13:06                                     ` Joakim Tjernlund
2009-10-02 18:10                                       ` Joakim Tjernlund
2009-10-02 21:49                                     ` Scott Wood
2009-10-02 22:04                                       ` Benjamin Herrenschmidt
2009-10-05 19:28                                         ` Scott Wood
2009-10-05 20:29                                           ` Benjamin Herrenschmidt
2009-10-05 21:04                                             ` Scott Wood
2009-10-03  8:05                                       ` Joakim Tjernlund
2009-10-03  8:31                                         ` Benjamin Herrenschmidt
2009-10-03  9:24                                           ` Joakim Tjernlund
2009-10-03 10:57                                             ` Benjamin Herrenschmidt
2009-10-03 11:47                                               ` Joakim Tjernlund
2009-10-04  8:35                                               ` Joakim Tjernlund
2009-10-04 20:26                                                 ` Benjamin Herrenschmidt
2009-10-04 20:38                                                   ` Joakim Tjernlund
2009-10-05 18:24                                         ` Scott Wood
2009-10-05 18:50                                           ` Joakim Tjernlund
2009-10-04 20:10                                       ` Joakim Tjernlund [this message]
2009-10-04 20:28                                         ` Benjamin Herrenschmidt
2009-10-04 20:45                                           ` Joakim Tjernlund
2009-10-05  7:28                                           ` Joakim Tjernlund
2009-10-05 19:16                                           ` Joakim Tjernlund
2009-10-05 20:28                                             ` Benjamin Herrenschmidt
2009-09-29  7:07             ` Benjamin Herrenschmidt
2009-09-29 21:09               ` Rex Feany

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=OF7C895670.A7F16A1A-ONC1257645.00620A4D-C1257645.006ED40F@transmode.se \
    --to=joakim.tjernlund@transmode.se \
    --cc=RFeany@mrv.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=scottwood@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.