LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Hugh Dickins <hughd@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Yang Shi <shy828301@gmail.com>, Miaohe Lin <linmiaohe@huawei.com>,
	Alistair Popple <apopple@nvidia.com>,
	Matthew Wilcox <willy@infradead.org>,
	David Hildenbrand <david@redhat.com>,
	Jerome Glisse <jglisse@redhat.com>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Liam Howlett <liam.howlett@oracle.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	peterx@redhat.com, Axel Rasmussen <axelrasmussen@google.com>
Subject: [PATCH v3 1/5] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte
Date: Wed,  8 Sep 2021 12:35:12 -0400	[thread overview]
Message-ID: <20210908163516.214441-2-peterx@redhat.com> (raw)
In-Reply-To: <20210908163516.214441-1-peterx@redhat.com>

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.

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.

Cc: Hugh Dickins <hughd@google.com>
Cc: Axel Rasmussen <axelrasmussen@google.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/shmem.c       | 1 -
 mm/userfaultfd.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 88742953532c..96ccf6e941aa 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2424,7 +2424,6 @@ int shmem_mfill_atomic_pte(struct mm_struct *dst_mm,
 	shmem_recalc_inode(inode);
 	spin_unlock_irq(&info->lock);
 
-	SetPageDirty(page);
 	unlock_page(page);
 	return 0;
 out_delete_from_cache:
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 7a9008415534..caf6dfff2a60 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -69,10 +69,9 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd,
 	pgoff_t offset, max_off;
 
 	_dst_pte = mk_pte(page, dst_vma->vm_page_prot);
+	_dst_pte = pte_mkdirty(_dst_pte);
 	if (page_in_cache && !vm_shared)
 		writable = false;
-	if (writable || !page_in_cache)
-		_dst_pte = pte_mkdirty(_dst_pte);
 	if (writable) {
 		if (wp_copy)
 			_dst_pte = pte_mkuffd_wp(_dst_pte);
-- 
2.31.1


  reply	other threads:[~2021-09-08 16:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 16:35 [PATCH v3 0/5] mm: A few cleanup patches around zap, shmem and uffd Peter Xu
2021-09-08 16:35 ` Peter Xu [this message]
2021-09-08 16:36 ` [PATCH v3 2/5] mm: Clear vmf->pte after pte_unmap_same() returns Peter Xu
2021-09-08 16:36 ` [PATCH v3 3/5] mm: Drop first_index/last_index in zap_details Peter Xu
2021-09-09  2:54   ` Liam Howlett
2021-09-09 18:13     ` Peter Xu
2021-09-08 16:36 ` [PATCH v3 4/5] mm: Add zap_skip_check_mapping() helper Peter Xu
2021-09-09  1:16   ` Alistair Popple
2021-09-08 16:36 ` [PATCH v3 5/5] mm: Add ZAP_FLAG_SKIP_SWAP and zap_flags Peter Xu
2021-09-15  2:25   ` Alistair Popple
2021-09-15  2:52     ` Peter Xu
2021-09-15  3:21       ` Alistair Popple
2021-09-15  4:01         ` Peter Xu
2021-09-14 16:37 ` [PATCH v3 0/5] mm: A few cleanup patches around zap, shmem and uffd Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210908163516.214441-2-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=axelrasmussen@google.com \
    --cc=david@redhat.com \
    --cc=hughd@google.com \
    --cc=jglisse@redhat.com \
    --cc=kirill@shutemov.name \
    --cc=liam.howlett@oracle.com \
    --cc=linmiaohe@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rppt@linux.vnet.ibm.com \
    --cc=shy828301@gmail.com \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH v3 1/5] mm/shmem: Unconditionally set pte dirty in mfill_atomic_install_pte' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).