Date:   Wed, 25 Jan 2023 15:45:29 +0100
In-Reply-To: <Y9A2PsCS7gfKWfaM@x1n>
Mime-Version: 1.0
References: <Y9A2PsCS7gfKWfaM@x1n>
Message-ID: <20230125144529.1630917-1-mdanylo@google.com>
Subject: Re: [PATCH v7 0/4] Implement IOCTL to get and/or the clear info about PTEs
From:   Danylo Mocherniuk <mdanylo@google.com>
To:     peterx@redhat.com
Cc:     Liam.Howlett@oracle.com, akpm@linux-foundation.org,
        alex.sierra@amd.com, avagin@gmail.com, axelrasmussen@google.com,
        brauner@kernel.org, dan.j.williams@intel.com, david@redhat.com,
        emmir@google.com, gorcunov@gmail.com, gregkh@linuxfoundation.org,
        gustavoars@kernel.org, kernel@collabora.com,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-mm@kvack.org,
        mdanylo@google.com, namit@vmware.com, pasha.tatashin@soleen.com,
        pgofman@codeweavers.com, rppt@kernel.org, shuah@kernel.org,
        shy828301@gmail.com, surenb@google.com, usama.anjum@collabora.com,
        vbabka@suse.cz, viro@zeniv.linux.org.uk, willy@infradead.org,
        yun.zhou@windriver.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jan 24, 2023 at 8:49 PM Peter Xu <peterx@redhat.com> wrote:
> On Mon, Jan 23, 2023 at 06:15:00PM +0500, Muhammad Usama Anjum wrote:
> > > Firstly, doc update is more than welcomed to explain the new interface
> > first (before throwing the code..).  That can be done in pagemap.rst on
> > > pagemap changes, or userfaultfd.rst on userfaultfd.
> > Okay. I'll add the documentation in next version or after the series has
> > been accepted. Initially I'd added the documentation. But the code kept on
> > changing so much that I had to spend considerable time on updating the
> > documentation. I know it is better to add documentation with the patches.
> > I'll try to add it.
>
> Yes, logically it should be the thing people start looking with.  It'll
> help reviewers to understand how does it work in general if relevant
> description is not in the cover letter, so it can matter even before the
> series is merged.
> > > There're four kinds of masks (required/anyof/excluded/return).  Are they
> > > all needed?  Why this is a good interface design?
> > Then, CRIU developers Andrea [1] and Danylo [2], asked to include all these
> > different kinds of masks. I'd thought of these masks as fancy filter inside
> > the kernel. But there wasn't anyone else to review. So I'd included them to
> > move forward. Please let me know your thoughts after reading emails from [1].
> The idea makes sense to me, thanks.  I just hope "moving it forward" is not
> the only reason that you included it.
> Please also consider to attach relevant links to your next cover letter so
> new reviewers can be aware of why the interface is proposed like that.
> IMHO it would be also great if the CRIU people can acknowledge the
> interface at some point to make sure it satisfies the needs.

I acknowledge that this interface looks good for my use case to get interesting
pages from a big sparse mapping. For Andrei's use case to iteratively dump memory
it also looks good IMO. 

> An POC would be even better on CRIU, but maybe that's asking too much.

Can't promise now, but happy to do this when there'll be a clear signal that this
patchset is about to be merged. Meanwhile, I'll make some smaller tests with this
patchset rebased and will get back if there are some problems with that. 

> -- 
> Peter Xu