On 9/19/19 1:25 AM, Wei Yang wrote: > Function p[um]dp_set_access_flags is used with update_mmu_cache_p[um]d > and the return value from p[um]dp_set_access_flags indicates whether it > is necessary to do the cache update.
If this change is correct, why was it not applied to ptep_set_access_flags()? That function has the same form. BTW, I rather dislike the 'dirty' variable name. It seems to be indicating whether the fault was a write fault or not and whether we *expect* the dirty bit to be set in 'entry'. > From current code logic, only when changed && dirty, related page table > entry would be updated. It is not necessary to update cache when the > real page table entry is not changed. This logic doesn't really hold up, though. If we are only writing accessed and/or dirty bits, then we *never* need to flush. The flush might avoid a stale TLB entry causing an extra page walk by the hardware, but it's probably not ever worth the cost of the flush. Unless there's something weird happening with paravirt, I can't ever see a good reason to flush the TLB when just setting accessed/dirty bits on bare-metal. This seems like a place where a debugging check to validate that only accessed/dirty bits are only being set would be a good idea.
