lkml.org 
[lkml]   [2023]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [v4 2/3] mm: Defer TLB flush by keeping both src and dst folios at migration
On Thu, Nov 09, 2023 at 01:59:07PM +0900, Byungchul Park wrote:
> +++ b/include/linux/page-flags.h
> @@ -136,6 +136,7 @@ enum pageflags {
> PG_arch_2,
> PG_arch_3,
> #endif
> + PG_migrc, /* Page is under migrc's control */
> __NR_PAGEFLAGS,

Yeah; no. We're out of page flags. And CXL is insufficiently
compelling to add more. If you use CXL, you don't care about
performance, by definition.

> @@ -589,6 +590,9 @@ TESTCLEARFLAG(Young, young, PF_ANY)
> PAGEFLAG(Idle, idle, PF_ANY)
> #endif
>
> +TESTCLEARFLAG(Migrc, migrc, PF_ANY)
> +__PAGEFLAG(Migrc, migrc, PF_ANY)

Why PF_ANY?

> +/*
> + * Initialize the page when allocated from buddy allocator.
> + */
> +static inline void migrc_init_page(struct page *p)
> +{
> + __ClearPageMigrc(p);
> +}

This flag should already be clear ... ?

> +/*
> + * Check if the folio is pending for TLB flush and then clear the flag.
> + */
> +static inline bool migrc_unpend_if_pending(struct folio *f)
> +{
> + return folio_test_clear_migrc(f);
> +}

If you named the flag better, you wouldn't need this wrapper.

> +static void migrc_mark_pending(struct folio *fsrc, struct folio *fdst)
> +{
> + folio_get(fsrc);
> + __folio_set_migrc(fsrc);
> + __folio_set_migrc(fdst);
> +}

This is almost certainly unsafe. By using the non-atomic bit ops, you
stand the risk of losing a simultaneous update to any other bit in this
word. Like, say, someone trying to lock the folio?

> +++ b/mm/page_alloc.c
> @@ -1535,6 +1535,9 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>
> set_page_owner(page, order, gfp_flags);
> page_table_check_alloc(page, order);
> +
> + for (i = 0; i != 1 << order; ++i)
> + migrc_init_page(page + i);

No.

\
 
 \ /
  Last update: 2023-11-20 13:54    [W:0.686 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site