lkml.org 
[lkml]   [2012]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 09/31] arm64: Cache maintenance routines
    Hi Simon,

    On Fri, Sep 07, 2012 at 08:35:42PM +0100, Simon Baatz wrote:
    > On Fri, Sep 07, 2012 at 05:26:44PM +0100, Catalin Marinas wrote:
    > > +#define ARCH_HAS_FLUSH_ANON_PAGE
    > > +static inline void flush_anon_page(struct vm_area_struct *vma,
    > > + struct page *page, unsigned long vmaddr)
    > > +{
    > > + extern void __flush_anon_page(struct vm_area_struct *vma,
    > > + struct page *, unsigned long);
    > > + if (PageAnon(page))
    > > + __flush_anon_page(vma, page, vmaddr);
    >
    >
    > __flush_anon_page() does nothing. Shouldn't this be removed as well?

    Yes, good point.

    > > +void __flush_dcache_page(struct address_space *mapping, struct page *page)
    > > +{
    > > + __flush_dcache_area(page_address(page), PAGE_SIZE);
    > > +}
    > > +
    > > +void __sync_icache_dcache(pte_t pte)
    > > +{
    > > + unsigned long pfn;
    > > + struct page *page;
    > > +
    > > + pfn = pte_pfn(pte);
    > > + if (!pfn_valid(pfn))
    > > + return;
    > > +
    > > + page = pfn_to_page(pfn);
    > > + if (!test_and_set_bit(PG_dcache_clean, &page->flags))
    > > + __flush_dcache_page(NULL, page);
    > > + __flush_icache_all();
    > > +}
    > > +
    > > +/*
    > > + * Ensure cache coherency between kernel mapping and userspace mapping of this
    > > + * page.
    > > + */
    > > +void flush_dcache_page(struct page *page)
    > > +{
    > > + struct address_space *mapping;
    > > +
    > > + /*
    > > + * The zero page is never written to, so never has any dirty cache
    > > + * lines, and therefore never needs to be flushed.
    > > + */
    > > + if (page == ZERO_PAGE(0))
    > > + return;
    > > +
    > > + mapping = page_mapping(page);
    > > +
    > > + if (mapping && !mapping_mapped(mapping))
    > > + clear_bit(PG_dcache_clean, &page->flags);
    > > + else {
    > > + __flush_dcache_page(mapping, page);
    > > + if (mapping)
    > > + __flush_icache_all();
    >
    >
    > Is this necessary to ensure I/D coherency? Then, I would have
    > expected
    >
    > if (mapping) {
    > __flush_dcache_page(mapping, page);
    > __flush_icache_all();
    > }
    >
    > similar to __sync_icache_dcache() above.

    We don't want to do additional flushing if !mapping_mapped() as the page
    isn't mapped in user space. In this case we defer the flushing until
    __sync_icache_dcache().

    The other case is for anonymous pages where mapping == NULL. Here we
    don't defer the D-cache flush and do it directly. The I-cache, if
    needed, is handled later in __sync_icache_dcache(). This was based on
    the idea that this case is mainly for the args/env page which is mapped
    shortly after anyway, so not worth deferring. On AArch64, I don't think
    it makes any difference. Maybe a slight improvement (at least in
    clarity) in flush_dcache_page():

    if (mapping && mapping_mapped(mapping)) {
    __flush_dcache_page(page);
    __flush_icache_all();
    set_bit(PG_dcache_clean, &page->flags);
    } else {
    clear_bit(PG_dcache_clean, &page->flags);
    }

    In this case the anonymous page flushing is deferred to
    __sync_icache_dcache().

    > What is the reason why the D-cache flush is done in different
    > cases than the following I-cache flush?

    For __sync_icache_dcache(), we need to handle the situation where the
    page mapped into user space has been cleaned (D-cache) but there may be
    stale data in the I-cache. I think this can only happen with an
    ASID-tagged VIVT I-cache configuration (which is allowed on AArch64) if
    an existing page has been unmapped and the same virtual address remapped
    (withing the same mm context) to a different page that had been cleaned
    previously. We could optimise the __sync_icache_dcache() as below:

    if (!test_and_set_bit(PG_dcache_clean, &page->flags)) {
    __flush_dcache_page(page);
    __flush_icache_all();
    } else if (icache_is_aivivt()) {
    __flush_icache_all();
    }

    > > + set_bit(PG_dcache_clean, &page->flags);
    > > + }
    > > +}
    > > +EXPORT_SYMBOL(flush_dcache_page);
    > > +
    > > +void __flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vmaddr)
    > > +{
    > > +}
    >
    > Note that the __flush_dcache_page(mapping, page) in
    > flush_dcache_page() above is called when page is an anonymous page
    > (since mapping == NULL in this case). If the call to
    > __flush_dcache_page() is right above, it should be needed
    > here as well?

    flush_anon_page() is called when the kernel needs to access an anonymous
    page. Given that the D-cache behaves like a PIPT, there is no need for
    additional flushing here. The flush_dcache_page() call was based on the
    assumption that such page needs flushing anyway and it's not worth
    deferring. But the code may be easier to understand as I suggested above
    (and slightly more optimal for the VIPT I-cache case).

    It looks like any other architecture does something different here.

    --
    Catalin


    \
     
     \ /
      Last update: 2012-09-12 12:21    [W:0.031 / U:0.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site