On Tue, Jun 05, 2018 at 12:53:57PM -0700, Nadav Amit wrote:
> While I do not have a specific reservation regarding the logic, I find the
> current TLB invalidation scheme hard to follow and inconsistent. I guess
> should_force_flush() can be extended and used more commonly to make things
> clearer.
>
> To be more specific and to give an example: Can should_force_flush() be used
> in zap_pte_range() to set the force_flush instead of the current code?
>
> if (!PageAnon(page)) {
> if (pte_dirty(ptent)) {
> force_flush = 1;
> ...
> }
>
That check is against !PageAnon pages where it's potentially critical
that the dirty PTE bit be propogated to the page. You could split the
separate the TLB flush from the dirty page setting but it's not the same
class of problem and without perf data, it's not clear it's worthwhile.
Note that I also didn't handle the huge page moving because it's already
naturally batching a larger range with a lower potential factor of TLB
flushing and has different potential race conditions.
I agree that the TLB handling would benefit from being simplier but it's
not a simple search/replace job to deal with the different cases that apply.
--
Mel Gorman
SUSE Labs
