lkml.org 
[lkml]   [2021]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/5] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte
From
Date
On 02.09.21 22:17, Peter Xu wrote:
> It was conditionally done previously, as there's one shmem special case that we
> use SetPageDirty() instead. However that's not necessary and it should be
> easier and cleaner to do it unconditionally in mfill_atomic_install_pte().
>
> The most recent discussion about this is here, where Hugh explained the history
> of SetPageDirty() and why it's possible that it's not required at all:
>
> https://lore.kernel.org/lkml/alpine.LSU.2.11.2104121657050.1097@eggly.anvils/
>
> Currently mfill_atomic_install_pte() has three callers:
>
> 1. shmem_mfill_atomic_pte
> 2. mcopy_atomic_pte
> 3. mcontinue_atomic_pte
>
> After the change: case (1) should have its SetPageDirty replaced by the dirty
> bit on pte (so we unify them together, finally), case (2) should have no
> functional change at all as it has page_in_cache==false, case (3) may add a
> dirty bit to the pte. However since case (3) is UFFDIO_CONTINUE for shmem,
> it's merely 100% sure the page is dirty after all, so should not make a real
> difference either.

Would it be worth adding VM_BUG_ON() to make sure that "100%" is really
the case?

>
> This should make it much easier to follow on which case will set dirty for
> uffd, as we'll simply set it all now for all uffd related ioctls. Meanwhile,
> no special handling of SetPageDirty() if there's no need.

To me this all sounds sane, but I'm certainly not an expert on that
code, so ...

--
Thanks,

David / dhildenb

\
 
 \ /
  Last update: 2021-09-03 09:44    [W:0.191 / U:1.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site