2023-07-27 12:00:55

by Muhammad Usama Anjum

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

On 7/27/23 4:26 PM, Michał Mirosław wrote:
> On Thu, 27 Jul 2023 at 10:03, Muhammad Usama Anjum
> <[email protected]> wrote:
>> On 7/27/23 2:10 AM, Michał Mirosław wrote:
>>> On Wed, 26 Jul 2023 at 10:34, Muhammad Usama Anjum
>>> <[email protected]> wrote:
>>>> On 7/25/23 11:05 PM, Michał Mirosław wrote:
>>>>> On Tue, 25 Jul 2023 at 11:11, Muhammad Usama Anjum
>>>>> <[email protected]> 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/[email protected]

> 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