Re: [v3] fs/proc/task_mmu: Implement IOCTL for efficient page table scanning

From: Muhammad Usama Anjum <usama.anjum@collabora.com>
To: "Michał Mirosław" <emmir@google.com>
Cc: "Muhammad Usama Anjum" <usama.anjum@collabora.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>,
	"Andrei Vagin" <avagin@gmail.com>,
	"Danylo Mocherniuk" <mdanylo@google.com>,
	"Alex Sierra" <alex.sierra@amd.com>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Axel Rasmussen" <axelrasmussen@google.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Cyrill Gorcunov" <gorcunov@gmail.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	"Gustavo A . R . Silva" <gustavoars@kernel.org>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Mike Rapoport" <rppt@kernel.org>,
	"Nadav Amit" <namit@vmware.com>,
	"Pasha Tatashin" <pasha.tatashin@soleen.com>,
	"Paul Gofman" <pgofman@codeweavers.com>,
	"Peter Xu" <peterx@redhat.com>, "Shuah Khan" <shuah@kernel.org>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Yang Shi" <shy828301@gmail.com>,
	"Yun Zhou" <yun.zhou@windriver.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	kernel@collabora.com
Subject: Re: [v3] fs/proc/task_mmu: Implement IOCTL for efficient page table scanning
Date: Thu, 27 Jul 2023 16:31:18 +0500	[thread overview]
Message-ID: <df71c1ca-0f3f-fffb-a5d8-14ea99577387@collabora.com> (raw)
In-Reply-To: <CABb0KFFnWVy5k+8DhoS6jJzqeDDMkt3u=Rj6KS2HQSz1BY1+bw@mail.gmail.com>

On 7/27/23 4:26 PM, Michał Mirosław wrote:
> On Thu, 27 Jul 2023 at 10:03, Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>> On 7/27/23 2:10 AM, Michał Mirosław wrote:
>>> On Wed, 26 Jul 2023 at 10:34, Muhammad Usama Anjum
>>> <usama.anjum@collabora.com> wrote:
>>>> On 7/25/23 11:05 PM, Michał Mirosław wrote:
>>>>> On Tue, 25 Jul 2023 at 11:11, Muhammad Usama Anjum
>>>>> <usama.anjum@collabora.com> wrote:
> [...]
>>>>> 2. For the address tagging part I'd prefer someone who knows how this
>>>>> is used take a look. We're ignoring the tag (but clear it on return in
>>>>> ->start) - so it doesn't matter for the ioctl() itself.
>>>> I've added Kirill if he can give his thoughts about tagged memory.
>>>>
>>>> Right now we are removing the tags from all 3 pointers (start, end, vec)
>>>> before using the pointers on kernel side. But we are overwriting and
>>>> writing the walk ending address in start which user can read/use.
>>>>
>>>> I think we shouldn't over-write the start (and its tag) and instead return
>>>> the ending walk address in new variable, walk_end.
>>>
>>> The overwrite of `start` is making the ioctl restart (continuation)
>>> easier to handle. I prefer the current way, but it's not a strong
>>> opinion.
>> We shouldn't overwrite the start if we aren't gonna put the correct tag. So
>> I've resorted to adding another variable `walk_end` to return the walk
>> ending address.
> 
> Yes. We have two options:
> 
> 1. add new field and have the userspace check it and update start
> itself to continue the scan,
I've selected this option and sent v26 already:
https://lore.kernel.org/all/20230727093637.1262110-1-usama.anjum@collabora.com

>  or:
> 2. reconstruct the tag from either orignal `start` or `end` and have
> the userspace re-set `start` if it wants to restart the scan instead
> of continuing.
In some case, compiler can put integrity checking metadata in the pointer's
upper byte. So copying start or end's meta data would be wrong.

> 
> (the second one, using `end`'s tag, might be the easiest for
> userspace, as it can check `start` == `end` when deciding to continue
> or restart).
> 
> Best Regards
> Michał Mirosław

-- 
BR,
Muhammad Usama Anjum