On Thu, Jan 30, 2025 at 02:03:42PM +0100, Simona Vetter wrote: > 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. Yeah, agree it should be a driver responsibility but also can't see how nouveau is correct there either. I might see if I can get it to blow up... > 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. I have vague recollections of that, but I could be confusing it with some of the migrate_vma_*() issues we had dropping dirty bits (see https://lkml.kernel.org/r/dd48e4882ce859c295c1a77612f66d198b0403f9.1662078528.git-series.apopple@xxxxxxxxxx) > -Sima > -- > Simona Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
