Hi
Am 16.03.26 um 10:36 schrieb Boris Brezillon:
[...]
This would keep the huge-page code within the first branch. And if it
fails, it still does the regular page fault.
Well, in practice that's not what we want anyway (see the other fix for
the huge_fault vs regular fault race), so I'd still be inclined to have
the folio_mark_accessed() handled in the common path and have the pmd
vs regular pfn insertion in some if/else branches. Something like that:
if (pmd_insert)
ret = drm_gem_shmem_try_insert_pfn_pmd(vmf, pfn);
else
ret = vmf_insert_pfn(vma, vmf->address, pfn);
if (ret == VM_FAULT_NOPAGE) {
folio_mark_accesed(folio);
/* Normal pages are mapped RO, and remapped RW afterwards. */
if (pmd_insert && vmf->flags & FAULT_FLAG_WRITE)
This style mixes conditions from different branching depths. The first
outermost branch uses pmd_insert to compute ret. Any then both have
changed places, so that ret is in the outer branch and pmd_insert is in
the inner branch. This is hard to maintain. It is already confusing now
and will be even more so to anyone locking at that code later on.
Best regards
Thomas
folio_mark_dirty(folio);
}
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
