On Thu, May 27, 2021 at 2:41 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> Peter,
>
> On Thu, May 27, 2021 at 12:04:53PM -0700, Peter Collingbourne wrote:
>
> [...]
>
> > +static bool may_avoid_write_fault(pte_t pte, struct vm_area_struct *vma,
> > + unsigned long cp_flags)
> > +{
> > + if (!(cp_flags & MM_CP_DIRTY_ACCT)) {
> > + if (!(vma_is_anonymous(vma) && (vma->vm_flags & VM_WRITE)))
> > + return false;
> > +
> > + if (page_count(pte_page(pte)) != 1)
> > + return false;
> > + }
>
> Can we make MM_CP_DIRTY_ACCT still in charge? IIUC that won't affect your use
> case, something like:
>
> /* Never apply trick if MM_CP_DIRTY_ACCT not set */
> if (!(cp_flags & MM_CP_DIRTY_ACCT))
> return false;
>
> The thing is that's really what MM_CP_DIRTY_ACCT is about, imho (as its name
> shows). Say, we should start to count on the dirty bit for applying the write
> bit only if that flag set. With above, I think we can drop the pte_uffd_wp()
> check below because uffd_wp never applies MM_CP_DIRTY_ACCT when do
> change_protection().
I don't think that would work. The anonymous pages that we're
interesting in optimizing are private writable pages, for which
vma_wants_writenotify(vma, vma->vm_page_prot) would return false (and
thus MM_CP_DIRTY_ACCT would not be set, and thus your code would
disable the optimization), because of this code at the top of
vma_wants_writenotify:
/* If it was private or non-writable, the write bit is already clear */
if ((vm_flags & (VM_WRITE|VM_SHARED)) != ((VM_WRITE|VM_SHARED)))
return 0;
IIUC, dirty accountable is about whether we can always apply the
optimization no matter what the ref count is, so it isn't suitable for
situations where we need to check the ref count.
Peter
