On 18.07.22 14:02, Nadav Amit wrote: > From: Nadav Amit <namit@xxxxxxxxxx> > > Anonymous pages might have the dirty bit clear, but this should not > prevent mprotect from making them writable if they are exclusive. > Therefore, skip the test whether the page is dirty in this case. > > Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Cc: Andy Lutomirski <luto@xxxxxxxxxx> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > Cc: David Hildenbrand <david@xxxxxxxxxx> > Cc: Peter Xu <peterx@xxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Cc: Will Deacon <will@xxxxxxxxxx> > Cc: Yu Zhao <yuzhao@xxxxxxxxxx> > Cc: Nick Piggin <npiggin@xxxxxxxxx> > Signed-off-by: Nadav Amit <namit@xxxxxxxxxx> > --- > mm/mprotect.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/mm/mprotect.c b/mm/mprotect.c > index 34c2dfb68c42..da5b9bf8204f 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -45,7 +45,7 @@ static inline bool can_change_pte_writable(struct vm_area_struct *vma, > > VM_BUG_ON(!(vma->vm_flags & VM_WRITE) || pte_write(pte)); > > - if (pte_protnone(pte) || !pte_dirty(pte)) > + if (pte_protnone(pte)) > return false; > > /* Do we need write faults for softdirty tracking? */ > @@ -66,7 +66,8 @@ static inline bool can_change_pte_writable(struct vm_area_struct *vma, > page = vm_normal_page(vma, addr, pte); > if (!page || !PageAnon(page) || !PageAnonExclusive(page)) > return false; > - } > + } else if (!pte_dirty(pte)) > + return false; > > return true; > } When I wrote that code, I was wondering how often that would actually happen in practice -- and if we care about optimizing that. Do you have a gut feeling in which scenarios this would happen and if we care? If the page is in the swapcache and was swapped out, you'd be requiring a writeback even though nobody modified the page and possibly isn't going to do so in the near future. -- Thanks, David / dhildenb
