All of lore.kernel.org
 help / color / mirror / Atom feed
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
To: Andrei Vagin <avagin@gmail.com>
Cc: usama.anjum@collabora.com, Jonathan Corbet <corbet@lwn.net>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:PROC FILESYSTEM" <linux-fsdevel@vger.kernel.org>,
	"open list:MEMORY MANAGEMENT" <linux-mm@kvack.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	kernel@collabora.com,
	Gabriel Krisman Bertazi <krisman@collabora.com>,
	David Hildenbrand <david@redhat.com>,
	Peter Enderborg <peter.enderborg@sony.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE
Date: Mon, 3 Oct 2022 16:21:22 +0500	[thread overview]
Message-ID: <2e1c33c8-a201-0f7f-17cf-22fec555c7ff@collabora.com> (raw)
In-Reply-To: <CANaxB-xDiTRCuWxBmrSH1u3e_ADbxCoQKmEoSsYfm4yW7k=v4A@mail.gmail.com>

On 9/28/22 10:24 PM, Andrei Vagin wrote:
> On Wed, Sep 21, 2022 at 11:26 AM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> Hi,
>>
>> Thank you for reviewing.
>>
>> On 9/19/22 7:58 PM, Andrei Vagin wrote:
>>>> This ioctl can be used by the CRIU project and other applications which
>>>> require soft-dirty PTE bit information. The following operations are
>>>> supported in this ioctl:
>>>> - Get the pages that are soft-dirty.
>>>
>>> I think this interface doesn't have to be limited by the soft-dirty
>>> bits only. For example, CRIU needs to know whether file, present and swap bits
>>> are set or not.
>> These operations can be performed by pagemap procfs file. Definitely
>> performing them through IOCTL will be faster. But I'm trying to add a
>> simple IOCTL by which some specific PTE bit can be read and cleared
>> atomically. This IOCTL can be extended to include other bits like file,
>> present and swap bits by keeping the interface simple. The following
>> mask advice is nice. But if we add that kind of masking, it'll start to
>> look like a filter on top of pagemap. My intention is to not duplicate
>> the functionality already provided by the pagemap. One may ask, then why
>> am I adding "get the soft-dirty pages" functionality? I'm adding it to
>> complement the get and clear operation. The "get" and "get and clear"
>> operations with special flag (PAGEMAP_SD_NO_REUSED_REGIONS) can give
>> results quicker by not splitting the VMAs.
> 
> This simple interface is good only for a limited number of use-cases.
> The interface
> that I suggest doesn't duplicate more code than this one, but it is much more
> universal. It will be a big mess if you add a separate API for each
> specific use-case.
>
>
>>> I mean we should be able to specify for what pages we need to get info
>>> for. An ioctl argument can have these four fields:
>>> * required bits (rmask & mask == mask) - all bits from this mask have to be set.
>>> * any of these bits (amask & mask != 0) - any of these bits is set.
>>> * exclude masks (emask & mask == 0) = none of these bits are set.
>>> * return mask - bits that have to be reported to user.
The required mask (rmask) makes sense to me. At the moment, I only know
about the practical use case for the required mask. Can you share how
can any and exclude masks help for the CRIU?

>>>
>>>> - Clear the pages which are soft-dirty.
>>>> - The optional flag to ignore the VM_SOFTDIRTY and only track per page
>>>> soft-dirty PTE bit
>>>>
>>>> There are two decisions which have been taken about how to get the output
>>>> from the syscall.
>>>> - Return offsets of the pages from the start in the vec
>>>
>>> We can conside to return regions that contains pages with the same set
>>> of bits.
>>>
>>> struct page_region {
>>>       void *start;
>>>       long size;
>>>       u64 bitmap;
>>> }
>>>
>>> And ioctl returns arrays of page_region-s. I believe it will be more
>>> compact form for many cases.
>> Thank you for mentioning this. I'd considered this while development.
>> But I gave up and used the simple array to return the offsets of the
>> pages as in the problem I'm trying to solve, the dirty pages may be
>> present amid non-dirty pages. The range may not be useful in that case.
> 
> This is a good example. If we expect more than two consequent pages
> on average, the "region" interface looks more prefered. I don't know your
> use-case, but in the case of CRIU, this assumption looks reasonable.
> 
>> Also we want to return only a specific number of pages of interest. The
>> following paragraph explains it.
>>
>>>
>>>> - Stop execution when vec is filled with dirty pages
>>>> These two arguments doesn't follow the mincore() philosophy where the
>>>> output array corresponds to the address range in one to one fashion, hence
>>>> the output buffer length isn't passed and only a flag is set if the page
>>>> is present. This makes mincore() easy to use with less control. We are
>>>> passing the size of the output array and putting return data consecutively
>>>> which is offset of dirty pages from the start. The user can convert these
>>>> offsets back into the dirty page addresses easily. Suppose, the user want
>>>> to get first 10 dirty pages from a total memory of 100 pages. He'll
>>>> allocate output buffer of size 10 and the ioctl will abort after finding the
>>>> 10 pages. This behaviour is needed to support Windows' getWriteWatch(). The
>>>> behaviour like mincore() can be achieved by passing output buffer of 100
>>>> size. This interface can be used for any desired behaviour.
> 
> Now, it is more clear where this interface came from. It repeats the interface
> of Windows' getWriteWatch. I think we have to look wider. The
> interface that reports
> regions will be more efficient for many use-cases. As for
> getWriteWatch, it will require
> a bit more code in user-space, but this code is trivial.
> 
> Thanks,
> Andrei

-- 
Muhammad Usama Anjum

  reply	other threads:[~2022-10-03 11:21 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-26  6:45 [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE Muhammad Usama Anjum
2022-08-26  6:45 ` [PATCH v3 1/4] fs/proc/task_mmu: update functions to clear the soft-dirty PTE bit Muhammad Usama Anjum
2022-08-26  6:45 ` [PATCH v3 2/4] fs/proc/task_mmu: Implement IOCTL to get and clear soft dirty " Muhammad Usama Anjum
2022-08-26  6:45 ` [PATCH v3 3/4] selftests: vm: add pagemap ioctl tests Muhammad Usama Anjum
2022-08-26  6:45 ` [PATCH v3 4/4] mm: add documentation of the new ioctl on pagemap Muhammad Usama Anjum
2022-08-26  8:22   ` Bagas Sanjaya
2022-09-07  9:40 ` [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE Muhammad Usama Anjum
2022-09-19 14:58 ` Andrei Vagin
2022-09-21 18:26   ` Muhammad Usama Anjum
2022-09-28 17:24     ` Andrei Vagin
2022-10-03 11:21       ` Muhammad Usama Anjum [this message]
2022-10-11  4:52         ` Andrei Vagin
2022-10-14 13:48           ` Danylo Mocherniuk
2022-10-18 10:36             ` Muhammad Usama Anjum
2022-10-18 10:48               ` Greg KH
2022-10-18 12:30                 ` Muhammad Usama Anjum
2022-10-18 11:11               ` Michał Mirosław
2022-10-18 13:22                 ` Muhammad Usama Anjum
2022-10-18 17:17                   ` Michał Mirosław
2022-10-19  7:18                     ` Muhammad Usama Anjum
2022-10-18 13:32               ` Muhammad Usama Anjum
2022-10-18 18:20                 ` Greg KH
2022-09-21 18:30 ` Muhammad Usama Anjum
2022-09-28  6:03 ` Muhammad Usama Anjum

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=2e1c33c8-a201-0f7f-17cf-22fec555c7ff@collabora.com \
    --to=usama.anjum@collabora.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@gmail.com \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@collabora.com \
    --cc=krisman@collabora.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peter.enderborg@sony.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.