On Thu, 15 Jun 2023 at 16:52, Michał Mirosław emmir@google.com wrote:
On Thu, 15 Jun 2023 at 15:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
I'll send next revision now. On 6/14/23 11:00 PM, Michał Mirosław wrote:
(A quick reply to answer open questions in case they help the next version.)
On Wed, 14 Jun 2023 at 19:10, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 6/14/23 8:14 PM, Michał Mirosław wrote:
On Wed, 14 Jun 2023 at 15:46, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 6/14/23 3:36 AM, Michał Mirosław wrote: > On Tue, 13 Jun 2023 at 12:29, Muhammad Usama Anjum > usama.anjum@collabora.com wrote: > For flags name: PM_REQUIRE_WRITE_ACCESS? > Or Is it intended to be checked only if doing WP (as the current name > suggests) and so it would be redundant as WP currently requires > `p->required_mask = PAGE_IS_WRITTEN`? This is intended to indicate that if userfaultfd is needed. If PAGE_IS_WRITTEN is mentioned in any of mask, we need to check if userfaultfd has been initialized for this memory. I'll rename to PM_SCAN_REQUIRE_UFFD.
Why do we need that check? Wouldn't `is_written = false` work for vmas not registered via uffd?
UFFD_FEATURE_WP_ASYNC and UNPOPULATED needs to be set on the memory region for it to report correct written values on the memory region. Without UFFD WP ASYNC and UNPOUPULATED defined on the memory, we consider UFFD_WP state undefined. If user hasn't initialized memory with UFFD, he has no right to set is_written = false.
How about calculating `is_written = is_uffd_registered() && is_uffd_wp()`? This would enable a user to apply GET+WP for the whole address space of a process regardless of whether all of it is registered.
I wouldn't want to check if uffd is registered again and again. This is why we are doing it only once every walk in pagemap_scan_test_walk().
There is no need to do the checks repeatedly. If I understand the code correctly, uffd registration is per-vma, so it can be communicated from test_walk to entry/hole callbacks via a field in pagemap_scan_private.
Actually... this could be exposed as a page category for the filter (e.g. PAGE_USES_UFFD_WP) and then you could just make the ioctl() to work for your usecase without tracking the ranges at the userspace side.
Best Regards Michał Mirosław