Re: [PATCH v3] mm/migrate: don't batch the TLB flush when there is nothing to batch

[Zhiteng Qiu]

On 11/9/26 14:07, David Hildenbrand (Arm) wrote:
> For the future, please don't send new revisions in reply to old revisions.

Sorry about that. I'll send the next revision as a new thread.

> Is there no way to avoid that boolean?

[...]

> This is all information the caller could provide, and the caller could just
> provide the TTU flag then instead of passing a new "bool".
>
> Also, likely this check can be put into a separate helper function.

Agreed. I'll move the decision into a helper called from
migrate_pages_batch() and pass the resulting flags to
migrate_folio_unmap().

> I am not quite happy about the folio_mapcount() check, though: as soon as we
> optimize try_to_unmap() to batch better for large folios, it would be just plain
> wrong.

That makes sense. A large folio's mapcount alone would not tell us
how many separate flushes are needed once its PTEs can be handled
in a batch.

Would it make sense to restrict the optimization to a single,
singly mapped small folio and keep batching for all large folios,
as in the scope of v2? The changelog performance numbers were
measured with the v2 check. Or do you have a different direction
in mind?

Moving the check to the caller would read the mapcount before
locking the folio. My understanding is that a concurrent change
could only affect which flush path is chosen, and both paths are
correct. Please let me know if I'm missing a concurrency concern.

Ying, since you suggested using folio_mapcount(), would you be
comfortable with limiting the optimization to small folios?

Zi, limiting it to small folios would also bring back
folio_maybe_mapped_shared(), as you suggested on v2, and I'll
shorten the comment there too. Sorry for not answering your review
of v2 before sending v3.

Thanks,
Zhiteng