Re: problems in commit 2d4dc890b5c8 (block: add helpers to run flush_dcache_page() against a bio and a request's pages)

[Russell King <rmk@arm.linux.org.uk>]

On Thu, Dec 10, 2009 at 12:20:55PM -0600, James Bottomley wrote:
> On Thu, 2009-12-10 at 18:06 +0000, Russell King wrote:
> > The above example code comes from non-aliasing VIPT - where for the
> > vast majority of cases, unmapping without flush is fine provided we
> > haven't written data.  However, unmapping with flush is required to
> > ensure coherency with the instruction cache.
> 
> right, but you can check those two cases in the unmap, can't you?

How?  (I'd have thought that would've been plainly obvious since I wrote
in the quoted bit below "_if_ you have such flags".)

> > > So I really think in kunmap(_atomic) we need to check to see if the page
> > > was modified (using the pte flags),
> > 
> > That's fine _if_ you have such flags.  Not everything has - in which
> > case, going down the route you're proposing means that every single
> > kunmap_atomic() ends up having to flush the whole page whether it's
> > actually needed on an architecture "just because" - with no technical
> > reason to actually do so.
> > 
> > We need the two cases separated for hardware which is not PARISC.
> 
> So having such a flag is a requirement of the linux mm code, isn't it?
> 
> I thought what you did on arm was mark the page read only (even if it's
> supposed to be read/write) and then trap on the write request and update
> the dirty bit and set the page to read/write ... don't you do that
> anymore?

We do that for user pages, and only user pages - it's partly maintained
by the generic kernel code, and partly by the page table attribute
translation.  We only make pages _user_ writable if they have the Linux
'write' and Linux 'dirty' bits set.  However, they remain writable from
normal kernel stores - but we use a special instruction to access them
which ensures that the user mode permissions get checked.

Essentially, the protections that the majority of ARM CPUs have available
to them are:

	User		Kernel
0:	No access	Read only
1:	No access	Read/write
2:	Read only	Read/write
3:	Read/write	Read/write

The logic we use for implementing the userspace dirty support switches
the page permissions between case 2 and 3 - which is going to be of no
use for kernel accesses.

Moreover, we don't map kernel RAM using 4K pages - we map it using 1MB
section mappings to save the TLB from being cycled through.  If we were
to apply the same principle there, we'd have to do it on a 1MB by 1MB
basis, or take an additional memory hit with TLB usage.

So, in order to have bits for each page, what you're asking for is:
- avoid using efficient section mappings which only use 1 level of
  page table, map everything using 2 levels of page tables using 4K
  pages.
- add code to handle an additional special "dirty" bit processing for
  kernel pages.

I think that is far too inefficient an option to even contemplate.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of: