Le 27/02/2023 à 21:20, Matthew Wilcox a écrit :
> On Mon, Feb 27, 2023 at 07:45:08PM +0000, Christophe Leroy wrote:
>> Hi,
>>
>> Le 27/02/2023 à 18:57, Matthew Wilcox (Oracle) a écrit :
>>> Add set_ptes(), update_mmu_cache_range() and flush_dcache_folio().
>>> Change the PG_arch_1 (aka PG_dcache_dirty) flag from being per-page to
>>> per-folio.
>>>
>>> I'm unsure about my merging of flush_dcache_icache_hugepage() and
>>> flush_dcache_icache_page() into flush_dcache_icache_folio() and subsequent
>>> removal of flush_dcache_icache_phys(). Please review.
>>
>> Not sure why you want to remove flush_dcache_icache_phys().
>
> Well, I didn't, necessarily. It's just that when I merged
> flush_dcache_icache_hugepage() and flush_dcache_icache_page()
> together, it was left with no callers.
>
>> Allthough that's only feasible when address bus is not wider than 32
>> bits and cannot be done on BOOKE as you can't switch off MMU on BOOKE,
>> flush_dcache_icache_phys() allows to flush not mapped pages without
>> having to map them. So it is more efficient.
>
> And it was just never done for the hugepage case?
I think on PPC32 hugepages are available only on 8xx and BOOKE. 8xx
doesn't have HIGHMEM and BOOKE cannot switch MMU off. So there is no use
case for flush_dcache_icache_phys() with hugepages.
>
>>> @@ -148,17 +103,20 @@ static void __flush_dcache_icache(void *p)
>>> invalidate_icache_range(addr, addr + PAGE_SIZE);
>>> }
>>>
>>> -static void flush_dcache_icache_hugepage(struct page *page)
>>> +void flush_dcache_icache_folio(struct folio *folio)
>>> {
>>> - int i;
>>> - int nr = compound_nr(page);
>>> + unsigned int i, nr = folio_nr_pages(folio);
>>>
>>> - if (!PageHighMem(page)) {
>>> + if (flush_coherent_icache())
>>> + return;
>>> +
>>> + if (!folio_test_highmem(folio)) {
>>> + void *addr = folio_address(folio);
>>> for (i = 0; i < nr; i++)
>>> - __flush_dcache_icache(lowmem_page_address(page + i));
>>> + __flush_dcache_icache(addr + i * PAGE_SIZE);
>>> } else {
>>> for (i = 0; i < nr; i++) {
>>> - void *start = kmap_local_page(page + i);
>>> + void *start = kmap_local_folio(folio, i * PAGE_SIZE);
>>>
>>> __flush_dcache_icache(start);
>>> kunmap_local(start);
>
> So you'd like this to be:
>
> } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) >
> sizeof(void *)) {
> for (i = 0; i < nr; i++) {
> void *start = kmap_local_folio(folio, i * PAGE_SIZE);
> __flush_dcache_icache(start);
> kunmap_local(start);
> }
> } else {
> unsigned long pfn = folio_pfn(folio);
> for (i = 0; i < nr; i++)
> flush_dcache_icache_phys((pfn + i) * PAGE_SIZE;
> }
>
> (or maybe you'd prefer a flush_dcache_icache_pfn() that doesn't need to
> worry about PAGE_MASK).
Yes looks good.
Christophe
