Re: [PATCH 6/7] arc: Implement the new page table range API

Matthew Wilcox
2023-02-13T15:16:35Z

On Mon, Feb 13, 2023 at 03:09:37AM +0000, Yin, Fengwei wrote:
> > +++ b/arch/arc/include/asm/cacheflush.h
> > @@ -25,17 +25,20 @@
> >   * in update_mmu_cache()
> >   */
> >  #define flush_icache_page(vma, page)
> > +#define flush_icache_pages(vma, page, nr)
> Maybe just remove these two definitions because general
> implementation is just no-op?

Then arc would have to include asm-generic/cacheflush.h and I don't
particularly want to debug any issues that might cause. This is
easier.

Long term, asm-generic/cacheflush.h's contents should be moved into
linux/cacheflush.h, but I've lacked the time to do that work.

To answer your question from the other email, the documentation says:

``void flush_icache_page(struct vm_area_struct *vma, struct page *page)``

All the functionality of flush_icache_page can be implemented in
flush_dcache_page and update_mmu_cache_range. In the future, the hope
is to remove this interface completely.

I'm not planning on doing that to an architecture that I'm not set up
to test ...

> > +void flush_dcache_page(struct page *page)
> > +{
> > +       return flush_dcache_folio(page_folio(page));
> > +}
> I am wondering whether we should add flush_dcache_folio_range()
> because it's possible just part of folio needs be flush. Thanks.

We could. I think it's up to the maintainers of architectures that
need their caches flushing to let us know what would be good for them.
Since I primarily work on x86, I have no personal desire to do this ;-)

One of the things that I've always found a little weird about
flush_dcache_page() (and now flush_dcache_folio()) is that it's used both
for flushing userspace writes (eg in filemap_read()) and for flushing
kernel writes (eg in __iomap_write_end()). Probably it was designed for
an architecture that flushes by physical address rather than by virtual.

Anyway, if we do have a flush_dcache_folio_kernel(), I'd like it
to take byte offsets. That would work well for __iomap_write_end();
it could be:

flush_dcache_folio_kernel(folio, offset_in_folio(folio, pos), len);

But I'm not volunteering to do this work.