Re: [PATCH V2 1/3] MIPS: Fix cache flushing for swap pages with non-DMA I/O.

[Zenon Fortuna <zenon.fortuna@imgtec.com>]

A small related chirp-in from the "lowly Linux userland":

Does the current system-call "cacheflush(2)" works with the newer kernels?
As the "man cacheflush" tells, it was supposed to work only on MIPS 
based systems.
In the past I had some problems with it, so used related YAMON functions 
(which worked
better).
Maybe it could be made working for MIPS again?
(is it supposed to work with current kernels?)
Cache flushing is a useful feature for some benchmarks (and customers 
had requested
related tests in the past).

        -zenon

Leonid Yegoshin wrote:
> On 02/20/2015 11:17 AM, Kevin Cernekee wrote:
>> On Thu, Feb 19, 2015 at 8:17 AM, Steven J. Hill 
>> <Steven.Hill@imgtec.com> wrote:
>>> From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
>>>
>>> Flush the D-cache before the page is given to a process
>>> as an executable (I-cache) page when the backing store
>>> is non-DMA I/O.
>>>
>>> Signed-off-by: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
>>> Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
>> This patch seems to make several different changes to the cache
>> maintenance code all at once:
>>
>> 1) Add logic to handle virtually tagged D$
> This is needed for 74K/1074K erratas. It is somehow was split from 
> 006a851b10a395955c153a145ad8241494d43688 which was accepted upstream 
> but erratas not go through.
>
> The HW behaviour is similar to virtually tagged D$ but it is a HW bug.
>
>>   and perform extra flushes
>> on TLB updates
> As I understand, it is about adding __update_cache() in 
> update_mmu_cache(), right?
> If so, then - this code exists from the first git version of kernel 
> from Mr. Linus.
> It was deleted by mistake, I think.
>
>>
>> 2) Add new write barriers betwen D$/I$ or D$/L2 flushes
> It is required. MIPS32/64 R2 specs say:
>
>> "For implementations which implement multiple level of caches without 
>> the inclusion property, the use of a SYNC
>> instruction after the CACHE instruction is still needed whenever 
>> writeback data has to be resident in the next level of
>> memory hierarchy."
>
> So, if we need to transfer instruction from D$ to L2 then we should 
> use SYNC after CACHE D$ before operates with this data in L2. In other 
> case the CACHE for L2 may go ahead of completion of D$ for the same 
> line and flush a stale data.
>
> The same is basically for transfer D$ --> I$ because in MIPS it is 
> done via L2 or memory.
>
>
>>
>> 3) Make __flush_anon_page() play nice with HIGHMEM on systems with 
>> cache aliases
>>
>> and maybe a few more that I missed.
> 4) It is basically a revert of patch 64f23ab30b1fe which kills a 
> performance in non Cavium Octeon CPUs, actually - any CPU which has no 
> D$ cache snooping in I$.
>
> But just revert is incorrect, the another proper stuff is required for 
> correct operations.
>>
>> Would it be possible to split this out into individual commits, and
>> include more comprehensive changelogs for each one describing the
>> exact problem being solved?
> I would ask Steven to do it. The original code dates back to 2.6.32 
> and it was packed/split in different patches multiple times. After a 
> lot of split/join following patch acceptance/rejection we have what we 
> have now.
>
> In my opinion, the process of splitting into individual commits are 
> something wrong - some patches are accepted and some - not, and we 
> have a buggy code now. It should be packed into functional patches but 
> each patch should be a single workable commit. If multiple patches are 
> needed to fix a problem then result is sometime wrong.
>
>>
>> Also, it would be helpful to clarify how this relates to the use of
>> swap (?) with a backing store that is non-DMA I/O.  Do you have an
>> example of a situation where the existing code broke?  A play-by-play
>> postmortem would make for interesting reading.
>
> I guess, it is a little incorrect - this code is REQUIRED for non-DMA 
> I/O with root FS or swap but it is not enough. Another patch is still 
> needed to complete, see http://patchwork.linux-mips.org/patch/8635/
>
> - Leonid.
>