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.
- 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