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 19:17:36 +0200	[thread overview]
Message-ID: <CABb0KFFGRgy9D212skxxFMsHV5n3qjqUP9d-bQaWLUmtH46H3A@mail.gmail.com> (raw)
In-Reply-To: <17d7d6f5-21dc-37e1-6843-29c77a0e14b6@collabora.com>

On Tue, 18 Oct 2022 at 15:23, Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> On 10/18/22 4:11 PM, Michał Mirosław wrote:
> > On Tue, 18 Oct 2022 at 12:36, Muhammad Usama Anjum
> > <usama.anjum@collabora.com> wrote:
[...]
> >>    * @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?
> Yes, it if for counting pages. As the regions can have multiple pages,
> user cannot specify through the number of regions that how many pages
> does he need. Page size is used here as well like the start and len.
> This is optional argument as this is only needed to emulate the Windows
> syscall getWriteWatch.

I'm wondering about the condition that max_out_page < vec_len. Since
both count different things (pages vs ranges) I would expect there is
no strict relation between them and information returned is as much as
fits both (IOW: at most vec_len ranges spanning not more than
max_out_page pages). The field's name and description I'd suggest
improving: maybe 'max_pages' with a comment that 0 = unlimited?

[...]
> >> /* Special flags */
> >> #define PAGEMAP_NO_REUSED_REGIONS       0x1
> >
> > What does this flag do?
> Some non-dirty pages get marked as dirty because of the kernel's
> internal activity. The dirty bit of the pages is stored in the VMA flags
> and in the per page flags. If any of these two bits are set, the page is
> considered to be dirty. Suppose you have cleared the dirty bit of half
> of VMA which will be done by splitting the VMA and clearing dirty flag
> in the half VMA and the pages in it. Now kernel may decide to merge the
> VMAs again as dirty bit of VMAs isn't considered if the VMAs should be
> merged. So the half VMA becomes dirty again. This splitting/merging
> costs performance. The application receives a lot of pages which aren't
> dirty in reality but marked as dirty. Performance is lost again here.
>
> This PAGEMAP_NO_REUSED_REGIONS flag is used to don't depend on the dirty
> flag in the VMA flags. It only depends on the individual page dirty bit.
> With doing this, the new memory regions which are just created, doesn't
> look like dirty when seen with the IOCTL, but look dirty when seen from
> pagemap. This seems okay as the user of this flag know the implication
> of using it.

Thanks for explaining! Could you include this as a comment in the patch?

Best Regards
Michał Mirosław

  reply	other threads:[~2022-10-18 17:18 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
2022-10-18 13:22                 ` Muhammad Usama Anjum
2022-10-18 17:17                   ` Michał Mirosław [this message]
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=CABb0KFFGRgy9D212skxxFMsHV5n3qjqUP9d-bQaWLUmtH46H3A@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.