From: Miaohe Lin <linmiaohe@huawei.com>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH v2 1/8] mm/swap: remember PG_anon_exclusive via a swp pte bit
Date: Wed, 13 Apr 2022 17:38:59 +0800 [thread overview]
Message-ID: <3b9c6cc6-c5f5-8a8d-0b0f-9ca903cfab20@huawei.com> (raw)
In-Reply-To: <374d2be1-e13d-e605-ff80-b9d5eee4c40e@redhat.com>
On 2022/4/13 17:30, David Hildenbrand wrote:
> On 13.04.22 10:58, Miaohe Lin wrote:
>> On 2022/3/30 0:43, David Hildenbrand wrote:
>>> Currently, we clear PG_anon_exclusive in try_to_unmap() and forget about
>> ...
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 14618f446139..9060cc7f2123 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -792,6 +792,11 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>>> &src_mm->mmlist);
>>> spin_unlock(&mmlist_lock);
>>> }
>>> + /* Mark the swap entry as shared. */
>>> + if (pte_swp_exclusive(*src_pte)) {
>>> + pte = pte_swp_clear_exclusive(*src_pte);
>>> + set_pte_at(src_mm, addr, src_pte, pte);
>>> + }
>>> rss[MM_SWAPENTS]++;
>>> } else if (is_migration_entry(entry)) {
>>> page = pfn_swap_entry_to_page(entry);
>>> @@ -3559,6 +3564,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>> struct page *page = NULL, *swapcache;
>>> struct swap_info_struct *si = NULL;
>>> rmap_t rmap_flags = RMAP_NONE;
>>> + bool exclusive = false;
>>> swp_entry_t entry;
>>> pte_t pte;
>>> int locked;
>>> @@ -3724,6 +3730,46 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>> BUG_ON(!PageAnon(page) && PageMappedToDisk(page));
>>> BUG_ON(PageAnon(page) && PageAnonExclusive(page));
>>>
>>> + /*
>>> + * Check under PT lock (to protect against concurrent fork() sharing
>>> + * the swap entry concurrently) for certainly exclusive pages.
>>> + */
>>> + if (!PageKsm(page)) {
>>> + /*
>>> + * Note that pte_swp_exclusive() == false for architectures
>>> + * without __HAVE_ARCH_PTE_SWP_EXCLUSIVE.
>>> + */
>>> + exclusive = pte_swp_exclusive(vmf->orig_pte);
>>> + if (page != swapcache) {
>>> + /*
>>> + * We have a fresh page that is not exposed to the
>>> + * swapcache -> certainly exclusive.
>>> + */
>>> + exclusive = true;
>>> + } else if (exclusive && PageWriteback(page) &&
>>> + !(swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
>>
>> Really sorry for late respond and a newbie question. IIUC, if SWP_STABLE_WRITES is set,
>> it means concurrent page modifications while under writeback is not supported. For these
>> problematic swap backends, exclusive marker is dropped. So the above if statement is to
>> filter out these problematic swap backends which have SWP_STABLE_WRITES set. If so, the
>> above check should be && (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)), i.e. no "!".
>> Or am I miss something?
>
> Oh, thanks for your careful eyes!
>
> Indeed, SWP_STABLE_WRITES indicates that the backend *requires* stable
> writes, meaning, we must not modify the page while writeback is active.
>
> So if and only if that is set, we must drop the exclusive marker.
>
> This essentially corresponds to previous reuse_swap_page() logic:
>
> bool reuse_swap_page(struct page *page)
> {
> ...
> if (!PageWriteback(page)) {
> ...
> } else {
> ...
> if (p->flags & SWP_STABLE_WRITES) {
> spin_unlock(&p->lock);
> return false;
> }
> ...
> }
>
> Fortunately, this only affects such backends. For backends without
> SWP_STABLE_WRITES, the current code is simply sub-optimal.
>
>
> So yes, this has to be
>
> } else if (exclusive && PageWriteback(page) &&
> (swp_swap_info(entry)->flags & SWP_STABLE_WRITES)) {
>
I am glad that my question helps. :)
>
> Let me try finding a way to test this, the tests I was running so far
> were apparently not using a backend with SWP_STABLE_WRITES.
>
That will be really helpful. Many thanks for your hard work!
next prev parent reply other threads:[~2022-04-13 9:39 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-29 16:43 [PATCH v2 0/8] mm: COW fixes part 3: reliable GUP R/W FOLL_GET of anonymous pages David Hildenbrand
2022-03-29 16:43 ` [PATCH v2 1/8] mm/swap: remember PG_anon_exclusive via a swp pte bit David Hildenbrand
2022-04-13 8:58 ` Miaohe Lin
2022-04-13 9:30 ` David Hildenbrand
2022-04-13 9:38 ` Miaohe Lin [this message]
2022-04-13 10:46 ` David Hildenbrand
2022-04-13 12:31 ` David Hildenbrand
2022-04-14 2:40 ` Miaohe Lin
2022-04-20 17:10 ` Vlastimil Babka
2022-04-20 17:13 ` David Hildenbrand
2022-03-29 16:43 ` [PATCH v2 2/8] mm/debug_vm_pgtable: add tests for __HAVE_ARCH_PTE_SWP_EXCLUSIVE David Hildenbrand
2022-04-20 17:14 ` Vlastimil Babka
2022-03-29 16:43 ` [PATCH v2 3/8] x86/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE David Hildenbrand
2022-04-19 12:46 ` David Hildenbrand
2022-03-29 16:43 ` [PATCH v2 4/8] arm64/pgtable: " David Hildenbrand
2022-03-29 16:43 ` [PATCH v2 5/8] s390/pgtable: cleanup description of swp pte layout David Hildenbrand
2022-03-30 16:48 ` Gerald Schaefer
2022-03-29 16:43 ` [PATCH v2 6/8] s390/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE David Hildenbrand
2022-03-30 16:48 ` Gerald Schaefer
2022-03-29 16:43 ` [PATCH v2 7/8] powerpc/pgtable: remove _PAGE_BIT_SWAP_TYPE for book3s David Hildenbrand
2022-03-30 6:07 ` Christophe Leroy
2022-03-30 6:58 ` David Hildenbrand
2022-03-29 16:43 ` [PATCH v2 8/8] powerpc/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE " David Hildenbrand
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=3b9c6cc6-c5f5-8a8d-0b0f-9ca903cfab20@huawei.com \
--to=linmiaohe@huawei.com \
--cc=david@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).