Date: Mon, 22 Mar 2010 11:26:09 -0500 (CDT) From: Mark Tinguely <tinguely@casselton.net> To: cognet@freebsd.org, tinguely@casselton.net Cc: freebsd-arm@freebsd.org Subject: Re: Performance of SheevaPlug on 8-stable Message-ID: <201003221626.o2MGQ9n8074143@casselton.net> In-Reply-To: <20100322150633.GA18277@ci0.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 22 Mar 2010 16:06:33 Olivier Houchard said:
> On Mon, Mar 22, 2010 at 09:55:04AM -0500, Mark Tinguely wrote:
> > On Mon, 08 Mar 2010 16:50:58, Grzegorz Bernacki said:
> >
> > > This is probably caused by mechanism which turns of cache for shared pages.
> > > When I add applied following path:
> > >
> > > diff --git a/sys/arm/arm/pmap.c b/sys/arm/arm/pmap.c
> > > index 390dc3c..d17c0cc 100644
> > > --- a/sys/arm/arm/pmap.c
> > > +++ b/sys/arm/arm/pmap.c
> > > @@ -1401,6 +1401,8 @@ pmap_fix_cache(struct vm_page *pg, pmap_t pm, vm_offset_t va)
> > > */
> > >
> > > TAILQ_FOREACH(pv, &pg->md.pv_list, pv_list) {
> > > + if (pv->pv_flags & PVF_EXEC)
> > > + return;
> > > /* generate a count of the pv_entry uses */
> > > if (pv->pv_flags & PVF_WRITE) {
> > > if (pv->pv_pmap == pmap_kernel())
> > >
> > > execution time of 'test' program is:
> > > mv78100-4# time ./test
> > > 5.000u 0.000s 0:05.40 99.8% 40+1324k 0+0io 0pf+0w
> > >
> > > and without this path is:
> > > mv78100-4# time ./test
> > > 295.000u 0.000s 4:56.01 99.7% 40+1322k 0+0io 0pf+0w
> > >
> > >
> > > I think we need to handle executable pages in different way.
> > >
> > > grzesiek
> >
> > Good going Oliver and thank-you on the pmap_enter_pv kernel map patch Revision
> > 205425.
> >
> > Last week, before this patch, Maks Verver was so kind to put some statements
> > in the VM (vm_page_free_toq()) for the SheevaPlug because I could not cause
> > these paths with the Gumstix emulator. Maks, could you add to
> > vm_phys_free_pages():
> >
> > if (m->md.pv_kva)
> > + {
> > + printf("vm_phys_free_pages: md.pv_kva 0x%08x\n", m->md.pv_kva);
> > m->md.pv_kva = 0;
> > + }
> >
> > Even on the Gumstix emulator with the current patch, pmap_fix_cache() still
> > has many executable pages that have both a kernel and user pv_entry. Looks
> > like something like the above patch is still needed.
> >
> I added a few printf and saw the same thing, however isn't assuming we
> shouldn't modify the cache settings if the page is executable a bit dangerous ?
> Or did I misread your patch ?
>
> Olivier
The pmap_fix_cache() PVF_EXEC is Grzegorz's patch. In his defense, he
later stated that we may need to flush the buffer. If the kernel map
is a read-in page, the DMA probably did a POST-READ flush; the user page
if was previously accessed - <thinking as I type: why should it?> - could
be stale and need to be invalidated.
I was mostly mentioning there is another problem here besides dangling
kernel allocations. Before I was pushing the removing the kernel allocation
in pv_kva as a good thing; not only hoping the kernel entries in the PVF_EXEC
were stale kernel entries, but also knowing that the removal of any stale
kernel entries would be good for future data mappings too.
Definitely, the kernel remap (more than 1 kernel mapping) case would
indicate we need to turn off the caches.
This situation could have been around for a while. Before FreeBSD 8.0, the
kernel maps did not have pv_entrys, and the cache fix routines did not count
them and we did not even know these user/kernel overlap existed.
--Mark Tinguely.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201003221626.o2MGQ9n8074143>