Properly implement flush_dcache_page in 2.4? (Or is it possible?)

Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Jun Sun]

My understanding is that if a page is mapped in both user space
and kernel space flush_dcache_page() is used to ensure those mappings
are consistent to each other.

In other words, if the page is modified in user space, kernel needs
to call flush_dcache_page() in order to see the change properly. 
Vice versa.

One immediate problem we have is that given the struct page
pointer argument to this function we have no way of knowing the user 
space virture address of that page (without searching through the whole
page table).  And worse, we might have multiple mappings of the same
page to different user processes at the different virtual addresses.

If a MIPS cpu has a virtually-indexed dcache and has cache aliasing 
problem, we need to know those user space vritual addresses
to flush this page properly.  I suspect some other CPU architectures 
should have this problem too.  True?

So my question is: how other CPU arches with the same problem
implement flush_dcache_page()?  Flushing the whole cache? Or
have a broken implementation and pretend it is OK?  :)

BTW, I assume this is not a big problem in 2.5 as we have reverse page
mapping.

Cheers.

Jun

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Russell King]

On Fri, May 30, 2003 at 10:32:54AM -0700, Jun Sun wrote:
> So my question is: how other CPU arches with the same problem
> implement flush_dcache_page()?  Flushing the whole cache? Or
> have a broken implementation and pretend it is OK?  :)

See __flush_dcache_page() in arch/arm/mm/fault-armv.c in 2.5.70.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Jun Sun]

On Fri, May 30, 2003 at 07:09:29PM +0100, Russell King wrote:
> On Fri, May 30, 2003 at 10:32:54AM -0700, Jun Sun wrote:
> > So my question is: how other CPU arches with the same problem
> > implement flush_dcache_page()?  Flushing the whole cache? Or
> > have a broken implementation and pretend it is OK?  :)
> 
> See __flush_dcache_page() in arch/arm/mm/fault-armv.c in 2.5.70.
>

Is this routine tested to be working?  At least passing a page
index as a full virtual address to flush_cache_page() looks suspicious.

In addition, I am not sure if the vma struct will show up in the
"shared" list _if_ the page is only mapped in one user process and
in kernel (for example, those pages you obtain through get_user_pages()
call).

I am not familiar with 2.5 kernel.  I was under impression that reverse
page mapping might provide an easy solution to this problem.

Jun

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Russell King]

On Fri, May 30, 2003 at 04:00:02PM -0700, Jun Sun wrote:
> Is this routine tested to be working?  At least passing a page
> index as a full virtual address to flush_cache_page() looks suspicious.

Well, given that it doesn't actually trip up any real life programs
(for me) its not that easy to say "yes, it works".  However, you are
correct, and the right flush_cache_page() call should be:

                flush_cache_page(mpnt, mpnt->vm_start + off << PAGE_SHIFT);

> In addition, I am not sure if the vma struct will show up in the
> "shared" list _if_ the page is only mapped in one user process and
> in kernel (for example, those pages you obtain through get_user_pages()
> call).

If a mapping is using MAP_SHARED, my understanding is that the pages should
appear on the i_mmap_shared list.

I don't see a reason to worry about privately mapped pages on the i_mmap
list since they are private, and therefore shouldn't be updated with
modifications to other mappings, which I'd have thought would include
writes to the file (although I'm not so sure atm.)

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Jun Sun]

On Sat, May 31, 2003 at 12:14:58AM +0100, Russell King wrote:
> > In addition, I am not sure if the vma struct will show up in the
> > "shared" list _if_ the page is only mapped in one user process and
> > in kernel (for example, those pages you obtain through get_user_pages()
> > call).
> 
> If a mapping is using MAP_SHARED, my understanding is that the pages should
> appear on the i_mmap_shared list.
>

That is my understanding too.
 
> I don't see a reason to worry about privately mapped pages on the i_mmap
> list since they are private, and therefore shouldn't be updated with
> modifications to other mappings, 

Actually there is, at least in 2.4.  Whenever kernel calls get_user_pages()
it maps a user page into kernel virtual address space.  If kernel modifies
that page, flush_dcache_page() needs to make sure any stale cache data
at user virtual address is flush in order user to see kernel changes.

I have a test case at 

	http://linux.junsun.net/test-programs

(Note, sometimes even if you pass the test, you _may_ still have a wrong
flush_dcache_page() implementation, because stale cache could be flushed 
due to other execution sequences)

I took a brief look of 2.5 code.  It seems this problem should still
exist (of course, assuming the CPU has cache aliasing problem and the
flush_dcache_page() is not properly implemented).  

Actually in 2.5 you may fail the test even if you have a properly implemented
flush_dcache_page().  It appears it lacks another flush_dcache_page()
after the direct_IO is done.  I don't have a working 2.5 on MIPS.  Can't 
verify that. 

Jun

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Hugh Dickins]

On Sat, 31 May 2003, Russell King wrote:
> 
> I don't see a reason to worry about privately mapped pages on the i_mmap
> list since they are private, and therefore shouldn't be updated with
> modifications to other mappings, which I'd have thought would include
> writes to the file (although I'm not so sure atm.)

Be not so sure.  vmas on the private i_mmap list can still contain
shared pages, which should see writes to the file; but of course their
already-COWed private pages won't see subsequent writes to the file.

Hugh

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Russell King]

On Sat, May 31, 2003 at 08:24:00AM +0100, Hugh Dickins wrote:
> On Sat, 31 May 2003, Russell King wrote:
> > 
> > I don't see a reason to worry about privately mapped pages on the i_mmap
> > list since they are private, and therefore shouldn't be updated with
> > modifications to other mappings, which I'd have thought would include
> > writes to the file (although I'm not so sure atm.)
> 
> Be not so sure.  vmas on the private i_mmap list can still contain
> shared pages, which should see writes to the file; but of course their
> already-COWed private pages won't see subsequent writes to the file.

Hmm, looking at the posix spec (do we follow POSIX for mmap?) the
behaviour of MAP_PRIVATE mappings when the underlying file is modified
is unspecified.

I guess missing the cache handling for such mappings fits the POSIX
spec, and is equally as yucky as the current behaviour on CPUs which
don't require these flushes.

(unless someone tells me that POSIX is on drugs, I'm not going to be
that bothered about the MAP_PRIVATE case.)

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Hugh Dickins]

On Sat, 31 May 2003, Russell King wrote:
> On Sat, May 31, 2003 at 08:24:00AM +0100, Hugh Dickins wrote:
> > On Sat, 31 May 2003, Russell King wrote:
> > > 
> > > I don't see a reason to worry about privately mapped pages on the i_mmap
> > > list since they are private, and therefore shouldn't be updated with
> > > modifications to other mappings, which I'd have thought would include
> > > writes to the file (although I'm not so sure atm.)
> > 
> > Be not so sure.  vmas on the private i_mmap list can still contain
> > shared pages, which should see writes to the file; but of course their
> > already-COWed private pages won't see subsequent writes to the file.
> 
> Hmm, looking at the posix spec (do we follow POSIX for mmap?) the
> behaviour of MAP_PRIVATE mappings when the underlying file is modified
> is unspecified.
> 
> I guess missing the cache handling for such mappings fits the POSIX
> spec, and is equally as yucky as the current behaviour on CPUs which
> don't require these flushes.
> 
> (unless someone tells me that POSIX is on drugs, I'm not going to be
> that bothered about the MAP_PRIVATE case.)

I was about to concede to you: just because the pages are shared doesn't
mean that _you_ have to be overanxious about immediately forcing changes
to be visible to userspace (though it would be unfriendly if updates were
to appear at lesser granularity than PAGE_SIZE: I've no idea whether
that's a possibility), the "unspecified" would allow that much - but
wouldn't allow you to show portions of entirely other files!

But I've just remembered the peculiar VM_SHARED handling in mm/mmap.c:
open a file O_RDONLY, mmap it PROT_READ MAP_SHARED, and the vma will be
put on the i_mmap list, not on the i_mmap_shared list.  So the i_mmap
list can actually contain shared mappings, not just private mappings.
Poor naming certainly: the i_mmap_shared list is for mappings though
which the object might be modified.

Hugh

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Russell King]

On Sat, May 31, 2003 at 09:33:04AM +0100, Hugh Dickins wrote:
> I was about to concede to you: just because the pages are shared doesn't
> mean that _you_ have to be overanxious about immediately forcing changes
> to be visible to userspace (though it would be unfriendly if updates were
> to appear at lesser granularity than PAGE_SIZE: I've no idea whether
> that's a possibility), the "unspecified" would allow that much - but
> wouldn't allow you to show portions of entirely other files!

Other files should not be stored in the same page though - if that's
happening today, then we have a violation of POSIX, wrong from Linus'
"quality of implementation" standpoint, and its a security hole.

> But I've just remembered the peculiar VM_SHARED handling in mm/mmap.c:
> open a file O_RDONLY, mmap it PROT_READ MAP_SHARED, and the vma will be
> put on the i_mmap list, not on the i_mmap_shared list.  So the i_mmap
> list can actually contain shared mappings, not just private mappings.
> Poor naming certainly: the i_mmap_shared list is for mappings though
> which the object might be modified.

Hmm, you're right here.  I wonder whether we could allow VM_SHARED to be
set on such mappings, thereby putting the pages on the i_mmap_shared
list rather than the i_mmap list.

The alternative is that we scan these two lists for every case which we
need to do something; this seems to make the split lists rather pointless
from an architecture implementors point of view.

Maybe some of the VM gurus can shed some more light on this subject?

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Properly implement flush_dcache_page in 2.4? (Or is it possible?)

[Hugh Dickins]

On Sat, 31 May 2003, Russell King wrote:
> On Sat, May 31, 2003 at 09:33:04AM +0100, Hugh Dickins wrote:
> > that's a possibility), the "unspecified" would allow that much - but
> > wouldn't allow you to show portions of entirely other files!
> 
> Other files should not be stored in the same page though - if that's
> happening today, then we have a violation of POSIX, wrong from Linus'
> "quality of implementation" standpoint, and its a security hole.

Sorry, false alarm, I didn't mean to imply that was happening anywhere:
I was just giving a throwaway example of how, though a standard might
say "unspecified", there are still limits to what's allowed - yes,
"quality of implementation" is a good measure.

Hugh