On 3/17/23 12:20 AM, Peter Xu wrote:
> Hello, Muhammad,
> 
> On Thu, Mar 09, 2023 at 06:57:12PM +0500, Muhammad Usama Anjum wrote:
>> Add new WP Async mode (UFFD_FEATURE_WP_ASYNC) which resolves the page
>> faults on its own. It can be used to track that which pages have been
>> written-to from the time the pages were write-protected. It is very
>> efficient way to track the changes as uffd is by nature pte/pmd based.
>>
>> UFFD synchronous WP sends the page faults to the userspace where the
>> pages which have been written-to can be tracked. But it is not efficient.
>> This is why this asynchronous version is being added. After setting the
>> WP Async, the pages which have been written to can be found in the pagemap
>> file or information can be obtained from the PAGEMAP_IOCTL.
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 
> Here's the patch that can enable WP_ASYNC for all kinds of memories (as I
> promised..).  Currently I only tested btrfs (besides the common three)
> which is the major fs I use locally, but I guess it'll also enable the rest
> no matter what's underneath, just like soft-dirty.
> 
> As I mentioned, I just feel it very unfortunate to have a lot of suffixes
> for the UFFD_FEATURE_* on types of memory, and I hope we get rid of it for
> this WP_ASYNC from the start because the workflow should really be similar
> to anon/shmem handling for most of the rest, just a few tweaks here and
> there.
> 
> I had a feeling that some type of special VMA will work weirdly, but let's
> see.. so far I don't come up with any.
> 
> If the patch looks fine to you, please consider replace this patch with
> patch 1 of mine where I attached.  Then patch 1 can be reviewed alongside
> with your series.
> 
> Logically patch 1 can be reviewed separately too, because it works
> perfectly afaiu without the atomic version of pagemap already.  But on my
> side I don't think it justifies anything really matters, so unless someone
> thinks it a good idea to post / review / merge it separately, you can keep
> that with your new pagemap ioctl.
> 
> Patch 2 is only for your reference.  It's not for merging quality so please
> don't put it into your series.  I do plan to cleanup the userfaultfd
> selftests in the near future first (when I wrote this I am more eager to do
> so..).  I also think your final pagemap test cases can cover quite a bit.
> 
> Thanks,
Thank you so much for the patch. I've tested hugetlb mem. This patch is
working fine for hugetlb shmem:
*shmid = shmget(2, size, SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
mem = shmat(*shmid, 0, 0);

I've found slight issue with hugetlb mem which has been mmaped:
mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
	   MAP_ANONYMOUS | MAP_HUGETLB | MAP_PRIVATE, -1, 0);
The issue is that even after witting to this memory, the wp flag is still
present there and memory doesn't appear to be dirty when it should have
been dirty. The temporary fix is to write to memory and write protect the
memory one extra time.

Here is how I'm checking if WP flag is set or not:
static inline bool is_huge_pte_uffd_wp(pte_t pte)
{
	return ((pte_present(pte) && huge_pte_uffd_wp(pte)) ||
		pte_swp_uffd_wp_any(pte));
}

I've isolated the reproducer inside kselftests by commenting the unrelated
code. Please have a look at the attached kselftest and follow from main or
search `//#define tmpfix` in the code.

-- 
BR,
Muhammad Usama Anjum