All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Mirosław" <emmir@google.com>
To: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: Danylo Mocherniuk <mdanylo@google.com>,
	avagin@gmail.com, linux-mm@kvack.org, akpm@linux-foundation.org,
	gregkh@linuxfoundation.org, corbet@lwn.net, david@redhat.com,
	kernel@collabora.com, krisman@collabora.com,
	linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	peter.enderborg@sony.com, shuah@kernel.org,
	viro@zeniv.linux.org.uk, willy@infradead.org, figiel@google.com,
	kyurtsever@google.com, Paul Gofman <pgofman@codeweavers.com>,
	surenb@google.com
Subject: Re: [PATCH v3 0/4] Implement IOCTL to get and clear soft dirty PTE
Date: Tue, 18 Oct 2022 13:11:25 +0200	[thread overview]
Message-ID: <CABb0KFGCm=K2X3-O=y3BJN85sT2C-y+XZRtLxnuabuOg+OrHwQ@mail.gmail.com> (raw)
In-Reply-To: <474513c0-4ff9-7978-9d77-839fe775d04c@collabora.com>

On Tue, 18 Oct 2022 at 12:36, Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
[...]
> I've included the masks which the CRIU developers have specified.
> max_out_page is another new optional variable which is needed to
> terminate the operation without visiting all the pages after finding the
> max_out_page number of desired pages. There is no way to terminate the
> operation without this variable.
>
> How does the interface looks now? Please comment.
>
> /* PAGEMAP IOCTL */
> #define PAGEMAP_GET             _IOWR('f', 16, struct pagemap_sd_args)
> #define PAGEMAP_CLEAR           _IOWR('f', 17, struct pagemap_sd_args)
> #define PAGEMAP_GET_AND_CLEAR   _IOWR('f', 18, struct pagemap_sd_args)

Why are three IOCTLs needed? Could CLEAR be a flag (like the
PAGEMAP_NO_REUSED_REGIONS) or 'cmask' and GET be implied when vec !=
NULL?

> /* Bits are set in the bitmap of the page_region and masks in
> pagemap_sd_args */
> #define PAGE_IS_SD      1 << 0
> #define PAGE_IS_FILE    1 << 1
> #define PAGE_IS_PRESENT 1 << 2
> #define PAGE_IS_SWAPED  1 << 3
>
> /**
>   * struct page_region - Page region with bitmap flags
>   * @start:     Start of the region
>   * @len:       Length of the region
>   * bitmap:     Bits sets for the region
>   */
> struct page_region {
>         __u64 start;
>         __u64 len;
>         __u64 bitmap;
> };

Could you explain what units start and len are using? Are they bytes
or pages (what size)?

> /**
>   * struct pagemap_sd_args - Soft-dirty IOCTL argument

Nit: it's not soft-dirty-specific anymore. Maybe "pagemap_scan_args"?

>   * @start:             Starting address
>   * @len:               Length of the region
>   * @vec:               Output page_region struct array
>   * @vec_len:           Length of the page_region struct array
>   * @max_out_page:      Optional max output pages (It must be less than
> vec_len if specified)

Why is it required to be less than vec_len? vec_len effectively
specifies max number of ranges to find, and this new additional field
counts pages, I suppose?
BTW, if we count pages, then what size of them? Maybe using bytes
(matching start/len fields) would be more consistent?

>   * @flags:             Special flags for the IOCTL
>   * @rmask:             Special flags for the IOCTL
>   * @amask:             Special flags for the IOCTL
>   * @emask:             Special flags for the IOCTL
>   * @__reserved:                Reserved member to preserve data alignment. Must be 0.
>   */
> struct pagemap_sd_args {
>         __u64 __user start;
>         __u64 len;
>         __u64 __user vec; // page_region
>         __u64 vec_len;    // sizeof(page_region)
>         __u32 flags;      // special flags
>         __u32 rmask;
>         __u32 amask;
>         __u32 emask;
>         __u32 max_out_page;
>         __u32 __reserved;
> };
>
> /* Special flags */
> #define PAGEMAP_NO_REUSED_REGIONS       0x1

What does this flag do?

Best Regards
Michał Mirosław

  parent reply	other threads:[~2022-10-18 11:11 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
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 [this message]
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='CABb0KFGCm=K2X3-O=y3BJN85sT2C-y+XZRtLxnuabuOg+OrHwQ@mail.gmail.com' \
    --to=emmir@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@gmail.com \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=figiel@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@collabora.com \
    --cc=krisman@collabora.com \
    --cc=kyurtsever@google.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=mdanylo@google.com \
    --cc=peter.enderborg@sony.com \
    --cc=pgofman@codeweavers.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=usama.anjum@collabora.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.