| Message ID | 20230825135918.4164671-12-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Rearrange batched folio freeing | expand |
On Fri, Aug 25, 2023 at 02:59:15PM +0100, Matthew Wilcox (Oracle) wrote: > Use free_unref_page_batch() to free the folios. This may increase > the numer of IPIs from calling try_to_unmap_flush() more often, > but that's going to be very workload-dependent. I'd like to propose this as a replacement for this patch. Queue the mapped folios up so we can flush them all in one go. Free the unmapped ones, and the mapped ones after the flush. It does change the ordering of mem_cgroup_uncharge_folios() and the page flush. I think that's OK. This is only build-tested; something has messed up my laptop and I can no longer launch VMs. diff --git a/mm/vmscan.c b/mm/vmscan.c index 6f13394b112e..526d5bb84622 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1706,14 +1706,16 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, struct pglist_data *pgdat, struct scan_control *sc, struct reclaim_stat *stat, bool ignore_references) { + struct folio_batch free_folios; LIST_HEAD(ret_folios); - LIST_HEAD(free_folios); + LIST_HEAD(mapped_folios); LIST_HEAD(demote_folios); unsigned int nr_reclaimed = 0; unsigned int pgactivate = 0; bool do_demote_pass; struct swap_iocb *plug = NULL; + folio_batch_init(&free_folios); memset(stat, 0, sizeof(*stat)); cond_resched(); do_demote_pass = can_demote(pgdat->node_id, sc); @@ -1723,7 +1725,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, struct address_space *mapping; struct folio *folio; enum folio_references references = FOLIOREF_RECLAIM; - bool dirty, writeback; + bool dirty, writeback, mapped = false; unsigned int nr_pages; cond_resched(); @@ -1957,6 +1959,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, stat->nr_lazyfree_fail += nr_pages; goto activate_locked; } + mapped = true; } /* @@ -2111,14 +2114,12 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, */ nr_reclaimed += nr_pages; - /* - * Is there need to periodically free_folio_list? It would - * appear not as the counts should be low - */ - if (unlikely(folio_test_large(folio))) - destroy_large_folio(folio); - else - list_add(&folio->lru, &free_folios); + if (mapped) { + list_add(&folio->lru, &mapped_folios); + } else if (folio_batch_add(&free_folios, folio) == 0) { + mem_cgroup_uncharge_folios(&free_folios); + free_unref_folios(&free_folios); + } continue; activate_locked_split: @@ -2182,9 +2183,22 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, pgactivate = stat->nr_activate[0] + stat->nr_activate[1]; - mem_cgroup_uncharge_list(&free_folios); try_to_unmap_flush(); - free_unref_page_list(&free_folios); + while (!list_empty(&mapped_folios)) { + struct folio *folio = list_first_entry(&mapped_folios, + struct folio, lru); + + list_del(&folio->lru); + if (folio_batch_add(&free_folios, folio) > 0) + continue; + mem_cgroup_uncharge_folios(&free_folios); + free_unref_folios(&free_folios); + } + + if (free_folios.nr) { + mem_cgroup_uncharge_folios(&free_folios); + free_unref_folios(&free_folios); + } list_splice(&ret_folios, folio_list); count_vm_events(PGACTIVATE, pgactivate);
On Mon, Sep 04, 2023 at 04:43:22AM +0100, Matthew Wilcox wrote: > On Fri, Aug 25, 2023 at 02:59:15PM +0100, Matthew Wilcox (Oracle) wrote: > > Use free_unref_page_batch() to free the folios. This may increase > > the numer of IPIs from calling try_to_unmap_flush() more often, > > but that's going to be very workload-dependent. > > I'd like to propose this as a replacement for this patch. Queue the > mapped folios up so we can flush them all in one go. Free the unmapped > ones, and the mapped ones after the flush. Any reaction to this patch? I'm putting together a v2 for posting after the merge window, and I got no feedback on whether the former version or this one is better. > It does change the ordering of mem_cgroup_uncharge_folios() and > the page flush. I think that's OK. This is only build-tested; > something has messed up my laptop and I can no longer launch VMs. > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 6f13394b112e..526d5bb84622 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1706,14 +1706,16 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > struct pglist_data *pgdat, struct scan_control *sc, > struct reclaim_stat *stat, bool ignore_references) > { > + struct folio_batch free_folios; > LIST_HEAD(ret_folios); > - LIST_HEAD(free_folios); > + LIST_HEAD(mapped_folios); > LIST_HEAD(demote_folios); > unsigned int nr_reclaimed = 0; > unsigned int pgactivate = 0; > bool do_demote_pass; > struct swap_iocb *plug = NULL; > > + folio_batch_init(&free_folios); > memset(stat, 0, sizeof(*stat)); > cond_resched(); > do_demote_pass = can_demote(pgdat->node_id, sc); > @@ -1723,7 +1725,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > struct address_space *mapping; > struct folio *folio; > enum folio_references references = FOLIOREF_RECLAIM; > - bool dirty, writeback; > + bool dirty, writeback, mapped = false; > unsigned int nr_pages; > > cond_resched(); > @@ -1957,6 +1959,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > stat->nr_lazyfree_fail += nr_pages; > goto activate_locked; > } > + mapped = true; > } > > /* > @@ -2111,14 +2114,12 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > */ > nr_reclaimed += nr_pages; > > - /* > - * Is there need to periodically free_folio_list? It would > - * appear not as the counts should be low > - */ > - if (unlikely(folio_test_large(folio))) > - destroy_large_folio(folio); > - else > - list_add(&folio->lru, &free_folios); > + if (mapped) { > + list_add(&folio->lru, &mapped_folios); > + } else if (folio_batch_add(&free_folios, folio) == 0) { > + mem_cgroup_uncharge_folios(&free_folios); > + free_unref_folios(&free_folios); > + } > continue; > > activate_locked_split: > @@ -2182,9 +2183,22 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > > pgactivate = stat->nr_activate[0] + stat->nr_activate[1]; > > - mem_cgroup_uncharge_list(&free_folios); > try_to_unmap_flush(); > - free_unref_page_list(&free_folios); > + while (!list_empty(&mapped_folios)) { > + struct folio *folio = list_first_entry(&mapped_folios, > + struct folio, lru); > + > + list_del(&folio->lru); > + if (folio_batch_add(&free_folios, folio) > 0) > + continue; > + mem_cgroup_uncharge_folios(&free_folios); > + free_unref_folios(&free_folios); > + } > + > + if (free_folios.nr) { > + mem_cgroup_uncharge_folios(&free_folios); > + free_unref_folios(&free_folios); > + } > > list_splice(&ret_folios, folio_list); > count_vm_events(PGACTIVATE, pgactivate); >
diff --git a/mm/vmscan.c b/mm/vmscan.c index 6f13394b112e..965c429847fd 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1706,14 +1706,15 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, struct pglist_data *pgdat, struct scan_control *sc, struct reclaim_stat *stat, bool ignore_references) { + struct folio_batch free_folios; LIST_HEAD(ret_folios); - LIST_HEAD(free_folios); LIST_HEAD(demote_folios); unsigned int nr_reclaimed = 0; unsigned int pgactivate = 0; bool do_demote_pass; struct swap_iocb *plug = NULL; + folio_batch_init(&free_folios); memset(stat, 0, sizeof(*stat)); cond_resched(); do_demote_pass = can_demote(pgdat->node_id, sc); @@ -2111,14 +2112,11 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, */ nr_reclaimed += nr_pages; - /* - * Is there need to periodically free_folio_list? It would - * appear not as the counts should be low - */ - if (unlikely(folio_test_large(folio))) - destroy_large_folio(folio); - else - list_add(&folio->lru, &free_folios); + if (folio_batch_add(&free_folios, folio) == 0) { + mem_cgroup_uncharge_folios(&free_folios); + try_to_unmap_flush(); + free_unref_folios(&free_folios); + } continue; activate_locked_split: @@ -2182,9 +2180,9 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, pgactivate = stat->nr_activate[0] + stat->nr_activate[1]; - mem_cgroup_uncharge_list(&free_folios); + mem_cgroup_uncharge_folios(&free_folios); try_to_unmap_flush(); - free_unref_page_list(&free_folios); + free_unref_folios(&free_folios); list_splice(&ret_folios, folio_list); count_vm_events(PGACTIVATE, pgactivate);
Use free_unref_page_batch() to free the folios. This may increase the numer of IPIs from calling try_to_unmap_flush() more often, but that's going to be very workload-dependent. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Cc: Mel Gorman <mgorman@suse.de> --- mm/vmscan.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)