On 2022-05-17 2:11 p.m., Helge Deller wrote:
On 5/17/22 16:26, John David Anglin wrote:
On 2022-05-17 9:24 a.m., Helge Deller wrote:
On 5/17/22 15:19, Rolf Eike Beer wrote:
Am Montag, 16. Mai 2022, 23:49:10 CEST schrieb Helge Deller:
On 5/16/22 23:28, Rolf Eike Beer wrote:
Am Montag, 16. Mai 2022, 17:14:47 CEST schrieb John David Anglin:
Originally, I was convinced that we needed to use tmpalias flushes
everwhere, for both user and kernel flushes. However, when I modified
flush_kernel_dcache_page_addr, to use a tmpalias flush, my c8000
would crash quite early when booting.
The PDC returns alias values of 0 for the icache and dcache. This
indicates that either the alias boundary is greater than 16MB or
equivalent aliasing doesn't work. I modified the tmpalias code to
make it easy to try alternate boundaries. I tried boundaries up to
128MB but still kernel tmpalias flushes didn't work on c8000.
This led me to conclude that tmpalias flushes don't work on PA8800
and PA8900 machines, and that we needed to flush directly using the
virtual address of user and kernel pages. This is likely the major
cause of instability on the c8000 and rp34xx machines.
Flushing user pages requires doing a temporary context switch as we
have to flush pages that don't belong to the current context. Further,
we have to deal with pages that aren't present. If a page isn't
present, the flush instructions fault on every line.
Other code has been rearranged and simplified based on testing. For
example, I introduced a flush_cache_dup_mm routine. flush_cache_mm
and flush_cache_dup_mm differ in that flush_cache_mm calls
purge_cache_pages and flush_cache_dup_mm calls flush_cache_pages.
In some implementations, pdc is more efficient than fdc. Based on
my testing, I don't believe there's any performance benefit on the
c8000.
V2:
1) Add flush_cache_page_check_pte routine.
2) Use it in copy_to_user_page and copy_from_user_page.
3) flush_anon_page moved to cache.c and updated.
4) Changed alias boundary to 64 MB for 64-bit kernels. Revised comment
regarding alias boundary for PA8800/PA8900 processors.
5) Removed struct mm_struct * argument from flush_cache_pages.
6) Fixed thinko in flush_cache_range. It increased the number of pages
flushed and slowed performance.
7) Removed sync changes from pacache.S.
V3:
1) copy_to_user_page and copy_from_user_page moved to cache.c to
improve inlining.
2) Replaced copy_user_page with copy_user_highpage.
3) Fixed cache threshold calculation on 32-bit kernels.
4) Don't warn on inequivalent private mappings in flush_dcache_page.
5) Return early from mm_total_size if size exceeds
parisc_cache_flush_threshold.
6) Disable non-access TLB warning in handle_nadtlb_fault. Warning
happens occassionally handling flushes for COW faults.
7) Remove flush_cache_dup_mm.
8) Flush entire cache in flush_cache_mm and flush_cache_range on
processors with aliasing caches. Only flush small cache ranges
on machines with PA8800/PA8900 processors.
9) Tested on rp3440, c8000 and c3750.
Given how long these changelogs are, and how fragile the whole caching is
I
think it is a good idea to split this patch into smaller ones, to improve
readability and being able to bisect it.
FWIW, I've done some cleanups to this patch and committed it to my for-next
tree. In case it's split up, please use the revised version.
Why did you modify get_ptep()? Until now it was just moved around in the file,
and IMHO it becomes less readable because all these needless variables are
batched up at the start of the function now. The only point I would see in
moving them all to the front is if there would be no nesting anymore, and
every condition was inverted:
Dave's original patch did not moved the variables to the beginning.
That change was me - just because checkpatch complained otherwise.
I agree that it's less readable.
The get_ptep code was originally based on the vmalloc_to_page code in vmalloc.c.
vmalloc_to_page code. This code has now changed, so I think get_ptep needs updating.
See get_old_pud in mremap.c. It looks good to me.
So, what's the consense of this discussion now?
I can easily split out the pr_warn("WARNING").
Moving the get_ptep() back to the original place seems ok, and I'll keep
the strange indenting which checkpatch want.
Is that ok?
It's okay with me. But maybe we need to add pgd_leaf and pgd_bad checks (etc). That's
what is concerning me.
Dave
--
John David Anglin dave.anglin@xxxxxxxx
