From: David Hildenbrand <david@redhat.com>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linux-kselftest@vger.kernel.org, sparclinux@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
"David S. Miller" <davem@davemloft.net>,
Peter Xu <peterx@redhat.com>, Hugh Dickins <hughd@google.com>,
Shuah Khan <shuah@kernel.org>, Yu Zhao <yuzhao@google.com>,
Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH v1 RESEND 3/6] sparc/mm: don't unconditionally set HW writable bit when setting PTE dirty on 64bit
Date: Wed, 12 Apr 2023 11:48:15 +0200 [thread overview]
Message-ID: <de93ddc4-29ff-6113-a146-bc278dcce5f9@redhat.com> (raw)
In-Reply-To: <20230411193548.GA2094947@ravnborg.org>
[...]
>> Let's fix the remaining issues and prepare for reverting the workarounds
>> by setting the HW writable bit only if both, the SW dirty bit and the SW
>> writable bit are set.
>>
>> We have to move pte_dirty() and pte_dirty() up. The code patching
> One of the pte_dirty() should be replaced with pte_write().
>
Indeed, thanks. I assume Andrew can change the latter to pte_write().
[unless I have to resend, of course]
> It would have been nice to separate moving and changes in two patches,
> but keeping it together works too.
I prefer to not have "move code within the same file around" in separate
patches as long as it doesn't add too much noise. Here, I think it's
acceptable.
>
>
>> mechanism and handling constants > 22bit is a bit special on sparc64.
>>
>> The ASM logic in pte_mkdirty() and pte_mkwrite() match the logic in
>> pte_mkold() to create the mask depending on the machine type. The ASM
>> logic in __pte_mkhwwrite() matches the logic in pte_present(), just
>> using an "or" instead of an "and" instruction.
>>
>> With this commit (sun4u in QEMU):
>> root@debian:~/linux/tools/testing/selftests/mm# ./mkdirty
>> # [INFO] detected THP size: 8192 KiB
>> TAP version 13
>> 1..6
>> # [INFO] PTRACE write access
>> ok 1 SIGSEGV generated, page not modified
>> # [INFO] PTRACE write access to THP
>> ok 2 SIGSEGV generated, page not modified
>> # [INFO] Page migration
>> ok 3 SIGSEGV generated, page not modified
>> # [INFO] Page migration of THP
>> ok 4 SIGSEGV generated, page not modified
>> # [INFO] PTE-mapping a THP
>> ok 5 SIGSEGV generated, page not modified
>> # [INFO] UFFDIO_COPY
>> ok 6 SIGSEGV generated, page not modified
>> # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
> Nice!
>
>>
>> This handling seems to have been in place forever.
>>
>> [1] https://lkml.kernel.org/r/533a7c3d-3a48-b16b-b421-6e8386e0b142@redhat.com
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>
> I tried to follow your changes, but my knowledge of gcc assembler failed
> me. But based on the nice and detailed change log and the code I managed
> to understand:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
Thanks Sam!
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2023-04-12 10:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-11 14:25 [PATCH v1 RESEND 0/6] mm: (pte|pmd)_mkdirty() should not unconditionally allow for write access David Hildenbrand
2023-04-11 14:25 ` [PATCH v1 RESEND 1/6] selftests/mm: reuse read_pmd_pagesize() in COW selftest David Hildenbrand
2023-04-11 14:25 ` [PATCH v1 RESEND 2/6] selftests/mm: mkdirty: test behavior of (pte|pmd)_mkdirty on VMAs without write permissions David Hildenbrand
2023-04-11 14:25 ` [PATCH v1 RESEND 3/6] sparc/mm: don't unconditionally set HW writable bit when setting PTE dirty on 64bit David Hildenbrand
2023-04-11 19:35 ` Sam Ravnborg
2023-04-12 9:48 ` David Hildenbrand [this message]
2023-04-12 21:15 ` Andrew Morton
2023-04-11 14:25 ` [PATCH v1 RESEND 4/6] mm/migrate: revert "mm/migrate: fix wrongly apply write bit after mkdirty on sparc64" David Hildenbrand
2023-04-11 14:25 ` [PATCH v1 RESEND 5/6] mm/huge_memory: revert "Partly revert "mm/thp: carry over dirty bit when thp splits on pmd"" David Hildenbrand
2023-04-11 14:25 ` [PATCH v1 RESEND 6/6] mm/huge_memory: conditionally call maybe_mkwrite() and drop pte_wrprotect() in __split_huge_pmd_locked() 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=de93ddc4-29ff-6113-a146-bc278dcce5f9@redhat.com \
--to=david@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=anshuman.khandual@arm.com \
--cc=davem@davemloft.net \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=peterx@redhat.com \
--cc=sam@ravnborg.org \
--cc=shuah@kernel.org \
--cc=sparclinux@vger.kernel.org \
--cc=yuzhao@google.com \
/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).