Skip to content

Commit

Permalink
mm: don't be stuck to rmap lock on reclaim path
Browse files Browse the repository at this point in the history
The rmap locks(i_mmap_rwsem and anon_vma->root->rwsem) could be contended
under memory pressure if processes keep working on their vmas(e.g., fork,
mmap, munmap).  It makes reclaim path stuck.  In our real workload traces,
we see kswapd is waiting the lock for 300ms+(worst case, a sec) and it
makes other processes entering direct reclaim, which were also stuck on
the lock.

This patch makes lru aging path try_lock mode like shink_page_list so the
reclaim context will keep working with next lru pages without being stuck.
if it found the rmap lock contended, it rotates the page back to head of
lru in both active/inactive lrus to make them consistent behavior, which
is basic starting point rather than adding more heristic.

Since this patch introduces a new "contended" field as out-param along
with try_lock in-param in rmap_walk_control, it's not immutable any longer
if the try_lock is set so remove const keywords on rmap related functions.
Since rmap walking is already expensive operation, I doubt the const
would help sizable benefit( And we didn't have it until 5.17).

In a heavy app workload in Android, trace shows following statistics.  It
almost removes rmap lock contention from reclaim path.

Martin Liu reported:

Before:

   max_dur(ms)  min_dur(ms)  max-min(dur)ms  avg_dur(ms)  sum_dur(ms)  count blocked_function
         1632            0            1631   151.542173        31672    209  page_lock_anon_vma_read
          601            0             601   145.544681        28817    198  rmap_walk_file

After:

   max_dur(ms)  min_dur(ms)  max-min(dur)ms  avg_dur(ms)  sum_dur(ms)  count blocked_function
          NaN          NaN              NaN          NaN          NaN    0.0             NaN
            0            0                0     0.127645            1     12  rmap_walk_file

[[email protected]: add comment, per Matthew]
  Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Minchan Kim <[email protected]>
Acked-by: Johannes Weiner <[email protected]>
Cc: Suren Baghdasaryan <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: John Dias <[email protected]>
Cc: Tim Murray <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: Martin Liu <[email protected]>
Cc: Minchan Kim <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
  • Loading branch information
minchank authored and akpm00 committed May 19, 2022
1 parent f4840cc commit 6d4675e
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 28 deletions.
5 changes: 5 additions & 0 deletions include/linux/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,11 @@ static inline void i_mmap_unlock_write(struct address_space *mapping)
up_write(&mapping->i_mmap_rwsem);
}

static inline int i_mmap_trylock_read(struct address_space *mapping)
{
return down_read_trylock(&mapping->i_mmap_rwsem);
}

static inline void i_mmap_lock_read(struct address_space *mapping)
{
down_read(&mapping->i_mmap_rwsem);
Expand Down
4 changes: 2 additions & 2 deletions include/linux/ksm.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ static inline void ksm_exit(struct mm_struct *mm)
struct page *ksm_might_need_to_copy(struct page *page,
struct vm_area_struct *vma, unsigned long address);

void rmap_walk_ksm(struct folio *folio, const struct rmap_walk_control *rwc);
void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc);
void folio_migrate_ksm(struct folio *newfolio, struct folio *folio);

#else /* !CONFIG_KSM */
Expand Down Expand Up @@ -79,7 +79,7 @@ static inline struct page *ksm_might_need_to_copy(struct page *page,
}

static inline void rmap_walk_ksm(struct folio *folio,
const struct rmap_walk_control *rwc)
struct rmap_walk_control *rwc)
{
}

Expand Down
28 changes: 20 additions & 8 deletions include/linux/rmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ static inline void anon_vma_lock_read(struct anon_vma *anon_vma)
down_read(&anon_vma->root->rwsem);
}

static inline int anon_vma_trylock_read(struct anon_vma *anon_vma)
{
return down_read_trylock(&anon_vma->root->rwsem);
}

static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
{
up_read(&anon_vma->root->rwsem);
Expand Down Expand Up @@ -366,37 +371,44 @@ int pfn_mkclean_range(unsigned long pfn, unsigned long nr_pages, pgoff_t pgoff,

void remove_migration_ptes(struct folio *src, struct folio *dst, bool locked);

/*
* Called by memory-failure.c to kill processes.
*/
struct anon_vma *folio_lock_anon_vma_read(struct folio *folio);
void page_unlock_anon_vma_read(struct anon_vma *anon_vma);
int page_mapped_in_vma(struct page *page, struct vm_area_struct *vma);

/*
* rmap_walk_control: To control rmap traversing for specific needs
*
* arg: passed to rmap_one() and invalid_vma()
* try_lock: bail out if the rmap lock is contended
* contended: indicate the rmap traversal bailed out due to lock contention
* rmap_one: executed on each vma where page is mapped
* done: for checking traversing termination condition
* anon_lock: for getting anon_lock by optimized way rather than default
* invalid_vma: for skipping uninterested vma
*/
struct rmap_walk_control {
void *arg;
bool try_lock;
bool contended;
/*
* Return false if page table scanning in rmap_walk should be stopped.
* Otherwise, return true.
*/
bool (*rmap_one)(struct folio *folio, struct vm_area_struct *vma,
unsigned long addr, void *arg);
int (*done)(struct folio *folio);
struct anon_vma *(*anon_lock)(struct folio *folio);
struct anon_vma *(*anon_lock)(struct folio *folio,
struct rmap_walk_control *rwc);
bool (*invalid_vma)(struct vm_area_struct *vma, void *arg);
};

void rmap_walk(struct folio *folio, const struct rmap_walk_control *rwc);
void rmap_walk_locked(struct folio *folio, const struct rmap_walk_control *rwc);
void rmap_walk(struct folio *folio, struct rmap_walk_control *rwc);
void rmap_walk_locked(struct folio *folio, struct rmap_walk_control *rwc);

/*
* Called by memory-failure.c to kill processes.
*/
struct anon_vma *folio_lock_anon_vma_read(struct folio *folio,
struct rmap_walk_control *rwc);
void page_unlock_anon_vma_read(struct anon_vma *anon_vma);

#else /* !CONFIG_MMU */

Expand Down
10 changes: 8 additions & 2 deletions mm/ksm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2610,7 +2610,7 @@ struct page *ksm_might_need_to_copy(struct page *page,
return new_page;
}

void rmap_walk_ksm(struct folio *folio, const struct rmap_walk_control *rwc)
void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
{
struct stable_node *stable_node;
struct rmap_item *rmap_item;
Expand All @@ -2634,7 +2634,13 @@ void rmap_walk_ksm(struct folio *folio, const struct rmap_walk_control *rwc)
struct vm_area_struct *vma;

cond_resched();
anon_vma_lock_read(anon_vma);
if (!anon_vma_trylock_read(anon_vma)) {
if (rwc->try_lock) {
rwc->contended = true;
return;
}
anon_vma_lock_read(anon_vma);
}
anon_vma_interval_tree_foreach(vmac, &anon_vma->rb_root,
0, ULONG_MAX) {
unsigned long addr;
Expand Down
2 changes: 1 addition & 1 deletion mm/memory-failure.c
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ static void collect_procs_anon(struct page *page, struct list_head *to_kill,
struct anon_vma *av;
pgoff_t pgoff;

av = folio_lock_anon_vma_read(folio);
av = folio_lock_anon_vma_read(folio, NULL);
if (av == NULL) /* Not actually mapped anymore */
return;

Expand Down
7 changes: 4 additions & 3 deletions mm/page_idle.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,12 @@ static bool page_idle_clear_pte_refs_one(struct folio *folio,
static void page_idle_clear_pte_refs(struct page *page)
{
struct folio *folio = page_folio(page);

/*
* Since rwc.arg is unused, rwc is effectively immutable, so we
* can make it static const to save some cycles and stack.
* Since rwc.try_lock is unused, rwc is effectively immutable, so we
* can make it static to save some cycles and stack.
*/
static const struct rmap_walk_control rwc = {
static struct rmap_walk_control rwc = {
.rmap_one = page_idle_clear_pte_refs_one,
.anon_lock = folio_lock_anon_vma_read,
};
Expand Down
52 changes: 41 additions & 11 deletions mm/rmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,9 +527,11 @@ struct anon_vma *page_get_anon_vma(struct page *page)
*
* Its a little more complex as it tries to keep the fast path to a single
* atomic op -- the trylock. If we fail the trylock, we fall back to getting a
* reference like with page_get_anon_vma() and then block on the mutex.
* reference like with page_get_anon_vma() and then block on the mutex
* on !rwc->try_lock case.
*/
struct anon_vma *folio_lock_anon_vma_read(struct folio *folio)
struct anon_vma *folio_lock_anon_vma_read(struct folio *folio,
struct rmap_walk_control *rwc)
{
struct anon_vma *anon_vma = NULL;
struct anon_vma *root_anon_vma;
Expand Down Expand Up @@ -557,6 +559,12 @@ struct anon_vma *folio_lock_anon_vma_read(struct folio *folio)
goto out;
}

if (rwc && rwc->try_lock) {
anon_vma = NULL;
rwc->contended = true;
goto out;
}

/* trylock failed, we got to sleep */
if (!atomic_inc_not_zero(&anon_vma->refcount)) {
anon_vma = NULL;
Expand Down Expand Up @@ -883,7 +891,8 @@ static bool invalid_folio_referenced_vma(struct vm_area_struct *vma, void *arg)
*
* Quick test_and_clear_referenced for all mappings of a folio,
*
* Return: The number of mappings which referenced the folio.
* Return: The number of mappings which referenced the folio. Return -1 if
* the function bailed out due to rmap lock contention.
*/
int folio_referenced(struct folio *folio, int is_locked,
struct mem_cgroup *memcg, unsigned long *vm_flags)
Expand All @@ -897,6 +906,7 @@ int folio_referenced(struct folio *folio, int is_locked,
.rmap_one = folio_referenced_one,
.arg = (void *)&pra,
.anon_lock = folio_lock_anon_vma_read,
.try_lock = true,
};

*vm_flags = 0;
Expand Down Expand Up @@ -927,7 +937,7 @@ int folio_referenced(struct folio *folio, int is_locked,
if (we_locked)
folio_unlock(folio);

return pra.referenced;
return rwc.contended ? -1 : pra.referenced;
}

static int page_vma_mkclean_one(struct page_vma_mapped_walk *pvmw)
Expand Down Expand Up @@ -2336,12 +2346,12 @@ void __put_anon_vma(struct anon_vma *anon_vma)
}

static struct anon_vma *rmap_walk_anon_lock(struct folio *folio,
const struct rmap_walk_control *rwc)
struct rmap_walk_control *rwc)
{
struct anon_vma *anon_vma;

if (rwc->anon_lock)
return rwc->anon_lock(folio);
return rwc->anon_lock(folio, rwc);

/*
* Note: remove_migration_ptes() cannot use folio_lock_anon_vma_read()
Expand All @@ -2353,7 +2363,17 @@ static struct anon_vma *rmap_walk_anon_lock(struct folio *folio,
if (!anon_vma)
return NULL;

if (anon_vma_trylock_read(anon_vma))
goto out;

if (rwc->try_lock) {
anon_vma = NULL;
rwc->contended = true;
goto out;
}

anon_vma_lock_read(anon_vma);
out:
return anon_vma;
}

Expand All @@ -2367,7 +2387,7 @@ static struct anon_vma *rmap_walk_anon_lock(struct folio *folio,
* contained in the anon_vma struct it points to.
*/
static void rmap_walk_anon(struct folio *folio,
const struct rmap_walk_control *rwc, bool locked)
struct rmap_walk_control *rwc, bool locked)
{
struct anon_vma *anon_vma;
pgoff_t pgoff_start, pgoff_end;
Expand Down Expand Up @@ -2415,7 +2435,7 @@ static void rmap_walk_anon(struct folio *folio,
* contained in the address_space struct it points to.
*/
static void rmap_walk_file(struct folio *folio,
const struct rmap_walk_control *rwc, bool locked)
struct rmap_walk_control *rwc, bool locked)
{
struct address_space *mapping = folio_mapping(folio);
pgoff_t pgoff_start, pgoff_end;
Expand All @@ -2434,8 +2454,18 @@ static void rmap_walk_file(struct folio *folio,

pgoff_start = folio_pgoff(folio);
pgoff_end = pgoff_start + folio_nr_pages(folio) - 1;
if (!locked)
if (!locked) {
if (i_mmap_trylock_read(mapping))
goto lookup;

if (rwc->try_lock) {
rwc->contended = true;
return;
}

i_mmap_lock_read(mapping);
}
lookup:
vma_interval_tree_foreach(vma, &mapping->i_mmap,
pgoff_start, pgoff_end) {
unsigned long address = vma_address(&folio->page, vma);
Expand All @@ -2457,7 +2487,7 @@ static void rmap_walk_file(struct folio *folio,
i_mmap_unlock_read(mapping);
}

void rmap_walk(struct folio *folio, const struct rmap_walk_control *rwc)
void rmap_walk(struct folio *folio, struct rmap_walk_control *rwc)
{
if (unlikely(folio_test_ksm(folio)))
rmap_walk_ksm(folio, rwc);
Expand All @@ -2468,7 +2498,7 @@ void rmap_walk(struct folio *folio, const struct rmap_walk_control *rwc)
}

/* Like rmap_walk, but caller holds relevant rmap lock */
void rmap_walk_locked(struct folio *folio, const struct rmap_walk_control *rwc)
void rmap_walk_locked(struct folio *folio, struct rmap_walk_control *rwc)
{
/* no ksm support for now */
VM_BUG_ON_FOLIO(folio_test_ksm(folio), folio);
Expand Down
7 changes: 6 additions & 1 deletion mm/vmscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,10 @@ static enum page_references folio_check_references(struct folio *folio,
if (vm_flags & VM_LOCKED)
return PAGEREF_ACTIVATE;

/* rmap lock contention: rotate */
if (referenced_ptes == -1)
return PAGEREF_KEEP;

if (referenced_ptes) {
/*
* All mapped folios start out with page table
Expand Down Expand Up @@ -2499,8 +2503,9 @@ static void shrink_active_list(unsigned long nr_to_scan,
}
}

/* Referenced or rmap lock contention: rotate */
if (folio_referenced(folio, 0, sc->target_mem_cgroup,
&vm_flags)) {
&vm_flags) != 0) {
/*
* Identify referenced, file-backed active pages and
* give them one more trip around the active list. So
Expand Down

0 comments on commit 6d4675e

Please sign in to comment.