lkml.org 
[lkml]   [2023]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC 2/2] mm: Defer TLB flush by keeping both src and dst folios at migration
Date


> On Aug 3, 2023, at 11:18 PM, Byungchul Park <byungchul@sk.com> wrote:
>
> Implementation of CONFIG_MIGRC that stands for 'Migration Read Copy'.
>
> We always face the migration overhead at either promotion or demotion,
> while working with tiered memory e.g. CXL memory and found out TLB
> shootdown is a quite big one that is needed to get rid of if possible.
>
> Fortunately, TLB flush can be defered or even skipped if both source and
> destination of folios during migration are kept until all TLB flushes
> required will have been done, of course, only if the target PTE entries
> have read only permission, more precisely speaking, don't have write
> permission. Otherwise, no doubt the folio might get messed up.
>
> To achieve that:
>
> 1. For the folios that have only non-writable TLB entries, prevent
> TLB flush by keeping both source and destination of folios during
> migration, which will be handled later at a better time.
>
> 2. When any non-writable TLB entry changes to writable e.g. through
> fault handler, give up CONFIG_MIGRC mechanism so as to perform
> TLB flush required right away.
>
> 3. TLB flushes can be skipped if all TLB flushes required to free the
> duplicated folios have been done by any reason, which doesn't have
> to be done from migrations.
>
> 4. Adjust watermark check routine, __zone_watermark_ok(), with the
> number of duplicated folios because those folios can be freed
> and obtained right away through appropreate TLB flushes.
>
> 5. Perform TLB flushes and free the duplicated folios pending the
> flushes if page allocation routine is in trouble due to memory
> pressure, even more aggresively for high order allocation.

So I think that what you want to do may be possible, but I think it worth
checking once an RFC that can be reviewed is posted. The complexity and
overheads would then need to be evaluated.

The patch in its current form, I am afraid, is very very hard to review.
It is way too big and is missing comments. Having CONFIG_MIGRC makes no
sense (I guess it is intended to be a “chicken-bit”). Variable and
function names are not informative. The memory barriers are handle
improperly (please check again the smp_mb__after_atomic() rules).

Actually, when it comes to concurrency, there are many things I did not
understand from a glance at the code when it comes to concurrency: the
use of llist_add when (I think?) the llist is not shared (I think?); the
use of WRITE_ONCE() for synchronization; migrc_gen scheme (and BTW, since
such a counter might overflow it should be atomic64).

But much more importantly, going up one level, there are several issues
that should be addressed/considered/discussed:

a. It seems to me that when a new PTE is established (e.g., following
an mmap()), and there are pending deferred flushes, a full TLB flush
would also be required. So your point (2) would need to be extended.

b. When a reference to the page is taken in other means (get_user_pages()),
a TLB flush might also be needed.

c. If we start deferring TLB flushes for a long time, and throughout that
time many events (TLB flush, page-faults, etc.) might require a *full*
TLB flush, that might have negative impact.

d. The interactions with other mechanisms that inspect the PTE to make
decisions and might not take into account the fact a TLB flush was not
done need to be considered. The interaction with mmu_gather has been
taken for, but there is a question of whether something here might
break it.

Now there are many things in the patch that need to be addressed and are
unacceptable in their current form (e.g., migrc_try_flush() flushing
potentially twice the same cores), but reviewing this patch in its
current form is too tedious for me.

[ BTW: for future versions, consider cc'ing Peter Zijlstra, Andy
Lutomirski and Dave Hansen. ]

\
 
 \ /
  Last update: 2023-08-04 19:32    [W:0.125 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site