Re: [PATCH 1/5] mm: Introduce mm_struct.has_pinned

Linus Torvalds
2020-09-27T18:45:53Z

On Sun, Sep 27, 2020 at 11:16 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Btw, I'm not convinced about the whole "turn the pte read-only and
> then back". If the fork races with another thread doing a pinning
> fast-GUP on another CPU, there are memory ordering issues etc too.
> That's not necessarily visible on x86 (the "turn read-only being a
> locked op will force serialization), but it all looks dodgy as heck.

.. looking at it more, I also think it could possibly lose the dirty
bit for the case where another CPU did a HW dirty/accessed bit update
in between the original read of the pte, and then us writing back the
writable pte again.

Us holding the page table lock means that no _software_ accesses will
happen to the PTE, but dirty/accessed bits can be modified by hardware
despite the lock.

That is, of course, a completely crazy case, and I think that since we
only do this for a COW mapping, and only do the PTE changes if the pte
was writable, the pte will always have been dirty already.

So I don't think it's an _actual_ bug, but it's another "this looks
dodgy as heck" marker. It may _work_, but it sure ain't pretty.

But despite having looked at this quite a bit, I don't see anything
that looks actively wrong, so I think the series is fine. This is more
of a note for people to perhaps think about.

Linus