Re: [patch] arch hook for notifying changes in PTE protections bits

Re: [patch] arch hook for notifying changes in PTE protections bits

[David S. Miller]

This is way overkill I think.

Take a look at set_pte_at().  You get the "mm", the
virtual address, the pte pointer, and the new pte value.

What else could you possibly need to track stuff like this
and react appropriately? :-)

It is even an argument for batched TLB processing on ia64.
It simplifies a lot of cache flushing issues and you can
control the flushing on a per-translation basis however
you like.

RE: [patch] arch hook for notifying changes in PTE protections bits

[Seth, Rohit]

David S. Miller <mailto:davem@davemloft.net> wrote on Friday, March 18,
2005 8:06 PM:

> 
> Take a look at set_pte_at().  You get the "mm", the
> virtual address, the pte pointer, and the new pte value.
> 

Thanks for pointing out the updated interface in 2.6.12-* kernel.  I
think I can overload the arch specific part of set_pte_at(or for that
matter set_pte, as what I need is only pte_t) to always check if we need
to do lazy I/D coherency for IA-64.....but this incurs extra cost for
making a check every time set_pte_at is called.  This new hook,
lazy_mmu_prot_update, though needs to be used only when the permissions
on a valid PTE is changing. For example, at the time of remap or swap,
this API is not called.
  
> What else could you possibly need to track stuff like this
> and react appropriately? :-)
> 

Stuff is there, though the call needs to be made to ensure we are
reacting to it most optimally and correctly.....I guess something
similar to why update_mmu_cache API is still existing in generic part
and not overloading arch specific set_pte_at definition.


-rohit

RE: [patch] arch hook for notifying changes in PTE protections bits

[David Mosberger]

>>>>> On Sat, 19 Mar 2005 12:22:20 -0800, "Seth, Rohit" <rohit.seth@intel.com> said:

  Rohit> David S. Miller <mailto:davem@davemloft.net> wrote on Friday,
  Rohit> March 18, 2005 8:06 PM:

  >>  Take a look at set_pte_at().  You get the "mm", the virtual
  >> address, the pte pointer, and the new pte value.


  Rohit> Thanks for pointing out the updated interface in 2.6.12-*
  Rohit> kernel.  I think I can overload the arch specific part of
  Rohit> set_pte_at(or for that matter set_pte, as what I need is only
  Rohit> pte_t) to always check if we need to do lazy I/D coherency
  Rohit> for IA-64.....but this incurs extra cost for making a check
  Rohit> every time set_pte_at is called.  This new hook,
  Rohit> lazy_mmu_prot_update, though needs to be used only when the
  Rohit> permissions on a valid PTE is changing. For example, at the
  Rohit> time of remap or swap, this API is not called.

Careful though: not every PTE has an associated page_map entry.

I agree about your concern about cost.  Accessing the page_map is
expensive (integer division + memory access) and we have to do that in
order to find out if the page is i-cache clean.

	--david

Re: [patch] arch hook for notifying changes in PTE protections bits

[David S. Miller]

On Sat, 19 Mar 2005 12:30:05 -0800
David Mosberger <davidm@napali.hpl.hp.com> wrote:

> I agree about your concern about cost.  Accessing the page_map is
> expensive (integer division + memory access) and we have to do that in
> order to find out if the page is i-cache clean.

First, it's a multiply by reciprocol.  At least on sparc64 I get
this emitted by the compiler.

Secondly, if you're willing to sacrifice 8 bytes per page struct
simply define WANT_PAGE_VIRTUAL and page struct will be exactly
64 bytes and thus the divide a will turn into a simple shift.

RE: [patch] arch hook for notifying changes in PTE protections bits

[Seth, Rohit]

David S. Miller <mailto:davem@davemloft.net> wrote on Saturday, March
19, 2005 4:27 PM:

> On Sat, 19 Mar 2005 12:30:05 -0800
> David Mosberger <davidm@napali.hpl.hp.com> wrote:
> 
>> I agree about your concern about cost.  Accessing the page_map is
>> expensive (integer division + memory access) and we have to do that
>> in order to find out if the page is i-cache clean.
> 
> First, it's a multiply by reciprocol.  At least on sparc64 I get
> this emitted by the compiler.
> 
> Secondly, if you're willing to sacrifice 8 bytes per page struct
> simply define WANT_PAGE_VIRTUAL and page struct will be exactly
> 64 bytes and thus the divide a will turn into a simple shift.

As said earlier, set_pte_at is getting called from different places.
Not all of these times, we are actually installing valid page entries in
page tables.  Validating the incoming entry for all of these scenarois
(or changing page struct) is an extra overhead that should be
avoided...besides being less palatable as compared to new API.

The recent changes in interfaces, because of set_pte_at, does provide an
opportunity though to handle mprotect issue little differently.  We
could call update_mmu_cache after setting the new protections in
change_pte_range function.....similar to the way update_mmu_cache gets
called at fault and any other times......though for this solution, we
need to extend change_pte_range interface to have vma parameter as well.



-rohit