To subscribe or unsubscribe via the World Wide Web, visit
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
or, via email, send a message with subject or body 'help' to
freebsd-a...@freebsd.org
You can reach the person managing the list at
freebsd-...@freebsd.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of freebsd-arm digest..."
Today's Topics:
1. Re: Performance of SheevaPlug on 8-stable (Mark Tinguely)
2. Re: Performance of SheevaPlug on 8-stable (Olivier Houchard)
3. Re: Performance of SheevaPlug on 8-stable (Mark Tinguely)
4. Re: Performance of SheevaPlug on 8-stable (Steve Woodford)
5. Re: Performance of SheevaPlug on 8-stable (Grzegorz Bernacki)
----------------------------------------------------------------------
Message: 1
Date: Mon, 22 Mar 2010 09:55:04 -0500 (CDT)
From: Mark Tinguely <ting...@casselton.net>
Subject: Re: Performance of SheevaPlug on 8-stable
To: g...@semihalf.com, ting...@casselton.net
Cc: freeb...@freebsd.org, cog...@freebsd.org
Message-ID: <201003221455....@casselton.net>
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.
--Mark Tinguely.
------------------------------
Message: 2
Date: Mon, 22 Mar 2010 16:06:33 +0100
From: Olivier Houchard <cog...@freebsd.org>
Subject: Re: Performance of SheevaPlug on 8-stable
To: Mark Tinguely <ting...@casselton.net>
Cc: freeb...@freebsd.org
Message-ID: <20100322150...@ci0.org>
Content-Type: text/plain; charset=us-ascii
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
------------------------------
Message: 3
Date: Mon, 22 Mar 2010 11:26:09 -0500 (CDT)
From: Mark Tinguely <ting...@casselton.net>
Subject: Re: Performance of SheevaPlug on 8-stable
To: cog...@freebsd.org, ting...@casselton.net
Cc: freeb...@freebsd.org
Message-ID: <201003221626....@casselton.net>
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.
------------------------------
Message: 4
Date: Tue, 23 Mar 2010 08:14:46 +0000
From: Steve Woodford <st...@mctavish.co.uk>
Subject: Re: Performance of SheevaPlug on 8-stable
To: freeb...@freebsd.org
Cc: Mark Tinguely <ting...@casselton.net>
Message-ID: <201003230814...@mctavish.co.uk>
Content-Type: text/plain; charset="iso-8859-1"
On Monday 22 March 2010 16:26:09 Mark Tinguely wrote:
> 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.
During development of the NetBSD/arm32 pmap (which FreeBSD now uses), I
noticed the same problem. The root cause was down in NetBSD's unified
buffer cache code, which mapped FS buffers writable by default.
The fix can be found by looking for PMAP_CACHE_VIVT in NetBSD's
sys/uvm/*
While FreeBSD doesn't use UVM, it might be useful to check if your VM
code needs a similar fix.
Steve
------------------------------
Message: 5
Date: Tue, 23 Mar 2010 12:14:12 +0100
From: Grzegorz Bernacki <g...@semihalf.com>
Subject: Re: Performance of SheevaPlug on 8-stable
To: Mark Tinguely <ting...@casselton.net>
Cc: freeb...@freebsd.org, cog...@freebsd.org
Message-ID: <4BA8A284...@semihalf.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Mark Tinguely wrote:
> 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.
Patch I've send is not a solution for this problem. I just send it to show
that excluding executable pages from fix_cache mechanism fixes the problem
and as I wrote in this mail, we need to handle executable pages with writable
kernel mapping differently.
I think that Mark is right. Kernel mapping should be already flushed out (we
can just do it again to make sure). I am not sure it there is any chance that
user mapping can have some cached data.
grzesiek
------------------------------
End of freebsd-arm Digest, Vol 208, Issue 2
*******************************************