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

[Michał Mirosław]

On Thu, 27 Jul 2023 at 10:03, Muhammad Usama Anjum
<usama.anjum@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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,
 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.

(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