On Thu, Jan 30, 2025 at 10:58:51AM +0100, David Hildenbrand wrote: > On 30.01.25 10:51, Simona Vetter wrote: > > On Wed, Jan 29, 2025 at 12:54:03PM +0100, David Hildenbrand wrote: > > > Let's do it just like mprotect write-upgrade or during NUMA-hinting > > > faults on PROT_NONE PTEs: detect if the PTE can be writable by using > > > can_change_pte_writable(). > > > > > > Set the PTE only dirty if the folio is dirty: we might not > > > necessarily have a write access, and setting the PTE writable doesn't > > > require setting the PTE dirty. > > > > Not sure whether there's much difference in practice, since a device > > exclusive access means a write, so the folio better be dirty (unless we > > aborted halfway through). But then I couldn't find the code in nouveau to > > do that, so now I'm confused. > > That confused me as well. Requiring the PTE to be writable does not imply > that it is dirty. > > So something must either set the PTE or the folio dirty. Yeah I'm not finding that something. > ( In practice, most anonymous folios are dirty most of the time ... ) And yup that's why I think it hasn't blown up yet. > If we assume that "device-exclusive entries" are always dirty, then it > doesn't make sense to set the folio dirty when creating device-exclusive > entries. We'd always have to set the PTE dirty when restoring the exclusive > pte. I do agree with your change, I think it's correct to put this responsibility onto drivers. It's just that nouveau seems to not be entirely correct. And thinking about this I have vague memories that I've discussed the case of the missing folio_set_dirty in noveau hmm code before, maybe with Alistair. But quick search in archives didn't turn up anything. -Sima -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
