Re: Happy New Year PARISC
|[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
On 1/3/2012 11:42 AM, James Bottomley wrote:
Note that I added flush code to "copy_user_page_asm" to handle this. I guess this flush could be avoided if we knew the page wasn't executable, but it is not that easy to figure out if a page is executable without the vma or mm. I also added 64-bit support and renamed the old version to copy_page_asm so both versions would beOn Tue, 2012-01-03 at 11:26 -0500, John David Anglin wrote:On 1/3/2012 10:32 AM, James Bottomley wrote:On Tue, 2012-01-03 at 10:13 -0500, John David Anglin wrote:On 1/3/2012 6:50 AM, Carlos O'Donell wrote:On Mon, Jan 2, 2012 at 6:12 PM, John David Anglin<dave.anglin@xxxxxxxx> wrote:None of this worked. Attached patch as it stands. Comments and testing appreciated.Could you clarify what you mean by "none of this worked?"I tried eliminating the flushes that occur in kunmap_atomic on PA8800 and PA8900 after the calls to clear_user_page and copy_user_page by defining clear_user_highpage and copy_user_highpage. I had thought the flushes weren't necessary. There's something about this that I don't understand. Why do we need to flush non-equivalent page mappings that aren't used?But they are used: Your work makes sure that all user space mappings are equivalent. However, because of the way Linux sets up kernel mappings (from the pfn array and offsets) the user virtual address and kernel virtual address almost never are. kmap is exclusively used so the kernel can access a user page, and at that point, we need to flush because we've set up an inequivalent alias (even if it's only done for read) kmap/kmap_atomic is used in more than just copy/flush ... or did you mean that you removed the kmap calls in copy/flush and the whole thing doesn't work (rather than as you imply you removed the flush in kunmap?)I didn't modify kmap/kunmap_atomic. I wrote versions of clear_user_highpage and copy_user_highpage to replace the default versions in linux/highmem.h. I replaced the kunmap_atomic calls with pagefault_enable to avoid the flush in the returns from clear/copy_user_page (actually, I only used one call to pagefault_enable, so maybe that was the issue). As far as I could tell, clear/copy_user_page are only called via clear/copy_user_highpage. The behavior of kmap/kunmap_atomic in other situations shouldn't have changed. Chapter F makes it clear that *all* inequivalent aliases to a page have to be removed when a write capable translation is enabled (no flush needed). When a write-capable translation needs to be read through an inequivalent alias, the page is supposed to be flushed, the write-capable translation is supposed to be removed from the page directory and then purged. That's why I added the purge_tlb_entries calls to set_pte_at and ptep_set_wrprotect. We avoid the flush by doing the `from' read through an equivalent mapping. However, the inequivalent mapping is still there. It seems to be necessary to purge the TLB entries prior to clearing/copying. However, from what I read in Chapter F, the purge is probably insufficient to speculative prevent move in. If I recall correctly, the kunmap_atomic also generates another TLB purge as well as a flush. There is a special access type (7) that can be used to prevent read and write move in.Actually, now I recall why copy_user_highpage never got implemented through the tmpalias space: it does cache hot copies (so we effectively copy straight from the cache of the source address into the cache of the destination). This is all fine and dandy and very fast until we have to copy executable pages: in this case, we set up an I/D cache inconsistency in userspace (which userspace apparently doesn't expect). It can be resolved by flushing the userspace cache, so the page becomes up to date an I movein sees the correct data, which is probably what the flush you still need is doing.
available for use/testing. I see that copy_user_highpage does have the vma, so maybe there is some hope of further optimizing copy_user_page_asm.I came to the conclusion that it was better to do a flush when copying through the tmpalias space than try to flush the `from' page in ptep_set_wrprotect as proposed by Niibe a couple of years ago. First, a COW page may never be written to, so why do a unnecessary flush (my assumption based on the documentation is that
clear/copy_user_page are primarily for COW support)?At the time the minifail bug was being discussed, I hadn't realized that the TLB
needed purging in set_pte_at and ptep_set_wrprotect. This became clear whenI looked at arm. Without the TLB purge, the flush proposed by Niibe was still
racy. Dave -- John David Anglin dave.anglin@xxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html