* [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-22 14:46 ` [PATCH 2/8] mm/vmscan: Throttle reclaim and compaction when too may pages are isolated Mel Gorman
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Page reclaim throttles on wait_iff_congested under the following conditions
o kswapd is encountering pages under writeback and marked for immediate
reclaim implying that pages are cycling through the LRU faster than
pages can be cleaned.
o Direct reclaim will stall if all dirty pages are backed by congested
inodes.
wait_iff_congested is almost completely broken with few exceptions. This
patch adds a new node-based workqueue and tracks the number of throttled
tasks and pages written back since throttling started. If enough pages
belonging to the node are written back then the throttled tasks will wake
early. If not, the throttled tasks sleeps until the timeout expires.
[neilb@suse.de: Uninterruptible sleep and simpler wakeups]
[hdanton@sina.com: Avoid race when reclaim starts]
[vbabka@suse.cz: vmstat irq-safe api, clarifications]
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/backing-dev.h | 1 -
include/linux/mmzone.h | 13 +++++
include/trace/events/vmscan.h | 34 +++++++++++++
include/trace/events/writeback.h | 7 ---
mm/backing-dev.c | 48 -------------------
mm/filemap.c | 1 +
mm/internal.h | 11 +++++
mm/page_alloc.c | 5 ++
mm/vmscan.c | 82 +++++++++++++++++++++++++++-----
mm/vmstat.c | 1 +
10 files changed, 135 insertions(+), 68 deletions(-)
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index ac7f231b8825..9fb1f0ae273c 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -154,7 +154,6 @@ static inline int wb_congested(struct bdi_writeback *wb, int cong_bits)
}
long congestion_wait(int sync, long timeout);
-long wait_iff_congested(int sync, long timeout);
static inline bool mapping_can_writeback(struct address_space *mapping)
{
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6a1d79d84675..419304093610 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -199,6 +199,7 @@ enum node_stat_item {
NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */
NR_DIRTIED, /* page dirtyings since bootup */
NR_WRITTEN, /* page writings since bootup */
+ NR_THROTTLED_WRITTEN, /* NR_WRITTEN while reclaim throttled */
NR_KERNEL_MISC_RECLAIMABLE, /* reclaimable non-slab kernel pages */
NR_FOLL_PIN_ACQUIRED, /* via: pin_user_page(), gup flag: FOLL_PIN */
NR_FOLL_PIN_RELEASED, /* pages returned via unpin_user_page() */
@@ -272,6 +273,11 @@ enum lru_list {
NR_LRU_LISTS
};
+enum vmscan_throttle_state {
+ VMSCAN_THROTTLE_WRITEBACK,
+ NR_VMSCAN_THROTTLE,
+};
+
#define for_each_lru(lru) for (lru = 0; lru < NR_LRU_LISTS; lru++)
#define for_each_evictable_lru(lru) for (lru = 0; lru <= LRU_ACTIVE_FILE; lru++)
@@ -841,6 +847,13 @@ typedef struct pglist_data {
int node_id;
wait_queue_head_t kswapd_wait;
wait_queue_head_t pfmemalloc_wait;
+
+ /* workqueues for throttling reclaim for different reasons. */
+ wait_queue_head_t reclaim_wait[NR_VMSCAN_THROTTLE];
+
+ atomic_t nr_writeback_throttled;/* nr of writeback-throttled tasks */
+ unsigned long nr_reclaim_start; /* nr pages written while throttled
+ * when throttling started. */
struct task_struct *kswapd; /* Protected by
mem_hotplug_begin/end() */
int kswapd_order;
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 88faf2400ec2..c317f9fe0d17 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -27,6 +27,14 @@
{RECLAIM_WB_ASYNC, "RECLAIM_WB_ASYNC"} \
) : "RECLAIM_WB_NONE"
+#define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
+
+#define show_throttle_flags(flags) \
+ (flags) ? __print_flags(flags, "|", \
+ {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"} \
+ ) : "VMSCAN_THROTTLE_NONE"
+
+
#define trace_reclaim_flags(file) ( \
(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
(RECLAIM_WB_ASYNC) \
@@ -454,6 +462,32 @@ DEFINE_EVENT(mm_vmscan_direct_reclaim_end_template, mm_vmscan_node_reclaim_end,
TP_ARGS(nr_reclaimed)
);
+TRACE_EVENT(mm_vmscan_throttled,
+
+ TP_PROTO(int nid, int usec_timeout, int usec_delayed, int reason),
+
+ TP_ARGS(nid, usec_timeout, usec_delayed, reason),
+
+ TP_STRUCT__entry(
+ __field(int, nid)
+ __field(int, usec_timeout)
+ __field(int, usec_delayed)
+ __field(int, reason)
+ ),
+
+ TP_fast_assign(
+ __entry->nid = nid;
+ __entry->usec_timeout = usec_timeout;
+ __entry->usec_delayed = usec_delayed;
+ __entry->reason = 1U << reason;
+ ),
+
+ TP_printk("nid=%d usec_timeout=%d usect_delayed=%d reason=%s",
+ __entry->nid,
+ __entry->usec_timeout,
+ __entry->usec_delayed,
+ show_throttle_flags(__entry->reason))
+);
#endif /* _TRACE_VMSCAN_H */
/* This part must be outside protection */
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 840d1ba84cf5..3bc759b81897 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -763,13 +763,6 @@ DEFINE_EVENT(writeback_congest_waited_template, writeback_congestion_wait,
TP_ARGS(usec_timeout, usec_delayed)
);
-DEFINE_EVENT(writeback_congest_waited_template, writeback_wait_iff_congested,
-
- TP_PROTO(unsigned int usec_timeout, unsigned int usec_delayed),
-
- TP_ARGS(usec_timeout, usec_delayed)
-);
-
DECLARE_EVENT_CLASS(writeback_single_inode_template,
TP_PROTO(struct inode *inode,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 4a9d4e27d0d9..0ea1a105eae5 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -1041,51 +1041,3 @@ long congestion_wait(int sync, long timeout)
return ret;
}
EXPORT_SYMBOL(congestion_wait);
-
-/**
- * wait_iff_congested - Conditionally wait for a backing_dev to become uncongested or a pgdat to complete writes
- * @sync: SYNC or ASYNC IO
- * @timeout: timeout in jiffies
- *
- * In the event of a congested backing_dev (any backing_dev) this waits
- * for up to @timeout jiffies for either a BDI to exit congestion of the
- * given @sync queue or a write to complete.
- *
- * The return value is 0 if the sleep is for the full timeout. Otherwise,
- * it is the number of jiffies that were still remaining when the function
- * returned. return_value == timeout implies the function did not sleep.
- */
-long wait_iff_congested(int sync, long timeout)
-{
- long ret;
- unsigned long start = jiffies;
- DEFINE_WAIT(wait);
- wait_queue_head_t *wqh = &congestion_wqh[sync];
-
- /*
- * If there is no congestion, yield if necessary instead
- * of sleeping on the congestion queue
- */
- if (atomic_read(&nr_wb_congested[sync]) == 0) {
- cond_resched();
-
- /* In case we scheduled, work out time remaining */
- ret = timeout - (jiffies - start);
- if (ret < 0)
- ret = 0;
-
- goto out;
- }
-
- /* Sleep until uncongested or a write happens */
- prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
- ret = io_schedule_timeout(timeout);
- finish_wait(wqh, &wait);
-
-out:
- trace_writeback_wait_iff_congested(jiffies_to_usecs(timeout),
- jiffies_to_usecs(jiffies - start));
-
- return ret;
-}
-EXPORT_SYMBOL(wait_iff_congested);
diff --git a/mm/filemap.c b/mm/filemap.c
index dae481293b5d..59187787fbfc 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1605,6 +1605,7 @@ void end_page_writeback(struct page *page)
smp_mb__after_atomic();
wake_up_page(page, PG_writeback);
+ acct_reclaim_writeback(page);
put_page(page);
}
EXPORT_SYMBOL(end_page_writeback);
diff --git a/mm/internal.h b/mm/internal.h
index cf3cb933eba3..b495e60c955d 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -34,6 +34,17 @@
void page_writeback_init(void);
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled);
+static inline void acct_reclaim_writeback(struct page *page)
+{
+ pg_data_t *pgdat = page_pgdat(page);
+ int nr_throttled = atomic_read(&pgdat->nr_writeback_throttled);
+
+ if (nr_throttled)
+ __acct_reclaim_writeback(pgdat, page, nr_throttled);
+}
+
vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b37435c274cf..78e538067651 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7389,6 +7389,8 @@ static void pgdat_init_kcompactd(struct pglist_data *pgdat) {}
static void __meminit pgdat_init_internals(struct pglist_data *pgdat)
{
+ int i;
+
pgdat_resize_init(pgdat);
pgdat_init_split_queue(pgdat);
@@ -7397,6 +7399,9 @@ static void __meminit pgdat_init_internals(struct pglist_data *pgdat)
init_waitqueue_head(&pgdat->kswapd_wait);
init_waitqueue_head(&pgdat->pfmemalloc_wait);
+ for (i = 0; i < NR_VMSCAN_THROTTLE; i++)
+ init_waitqueue_head(&pgdat->reclaim_wait[i]);
+
pgdat_page_ext_init(pgdat);
lruvec_init(&pgdat->__lruvec);
}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 74296c2d1fed..0c1595065f49 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1006,6 +1006,64 @@ static void handle_write_error(struct address_space *mapping,
unlock_page(page);
}
+static void
+reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
+ long timeout)
+{
+ wait_queue_head_t *wqh = &pgdat->reclaim_wait[reason];
+ long ret;
+ DEFINE_WAIT(wait);
+
+ /*
+ * Do not throttle IO workers, kthreads other than kswapd or
+ * workqueues. They may be required for reclaim to make
+ * forward progress (e.g. journalling workqueues or kthreads).
+ */
+ if (!current_is_kswapd() &&
+ current->flags & (PF_IO_WORKER|PF_KTHREAD))
+ return;
+
+ if (atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
+ WRITE_ONCE(pgdat->nr_reclaim_start,
+ node_page_state(pgdat, NR_THROTTLED_WRITTEN));
+ }
+
+ prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+ ret = schedule_timeout(timeout);
+ finish_wait(wqh, &wait);
+ atomic_dec(&pgdat->nr_writeback_throttled);
+
+ trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
+ jiffies_to_usecs(timeout - ret),
+ reason);
+}
+
+/*
+ * Account for pages written if tasks are throttled waiting on dirty
+ * pages to clean. If enough pages have been cleaned since throttling
+ * started then wakeup the throttled tasks.
+ */
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled)
+{
+ unsigned long nr_written;
+
+ inc_node_page_state(page, NR_THROTTLED_WRITTEN);
+
+ /*
+ * This is an inaccurate read as the per-cpu deltas may not
+ * be synchronised. However, given that the system is
+ * writeback throttled, it is not worth taking the penalty
+ * of getting an accurate count. At worst, the throttle
+ * timeout guarantees forward progress.
+ */
+ nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
+ READ_ONCE(pgdat->nr_reclaim_start);
+
+ if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
+ wake_up(&pgdat->reclaim_wait[VMSCAN_THROTTLE_WRITEBACK]);
+}
+
/* possible outcome of pageout() */
typedef enum {
/* failed to write page out, page is locked */
@@ -1412,9 +1470,8 @@ static unsigned int shrink_page_list(struct list_head *page_list,
/*
* The number of dirty pages determines if a node is marked
- * reclaim_congested which affects wait_iff_congested. kswapd
- * will stall and start writing pages if the tail of the LRU
- * is all dirty unqueued pages.
+ * reclaim_congested. kswapd will stall and start writing
+ * pages if the tail of the LRU is all dirty unqueued pages.
*/
page_check_dirty_writeback(page, &dirty, &writeback);
if (dirty || writeback)
@@ -3180,19 +3237,19 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
* If kswapd scans pages marked for immediate
* reclaim and under writeback (nr_immediate), it
* implies that pages are cycling through the LRU
- * faster than they are written so also forcibly stall.
+ * faster than they are written so forcibly stall
+ * until some pages complete writeback.
*/
if (sc->nr.immediate)
- congestion_wait(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
}
/*
- * Tag a node/memcg as congested if all the dirty pages
- * scanned were backed by a congested BDI and
- * wait_iff_congested will stall.
+ * Tag a node/memcg as congested if all the dirty pages were marked
+ * for writeback and immediate reclaim (counted in nr.congested).
*
* Legacy memcg will stall in page writeback so avoid forcibly
- * stalling in wait_iff_congested().
+ * stalling in reclaim_throttle().
*/
if ((current_is_kswapd() ||
(cgroup_reclaim(sc) && writeback_throttling_sane(sc))) &&
@@ -3200,15 +3257,15 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
set_bit(LRUVEC_CONGESTED, &target_lruvec->flags);
/*
- * Stall direct reclaim for IO completions if underlying BDIs
- * and node is congested. Allow kswapd to continue until it
+ * Stall direct reclaim for IO completions if the lruvec is
+ * node is congested. Allow kswapd to continue until it
* starts encountering unqueued dirty pages or cycling through
* the LRU too quickly.
*/
if (!current_is_kswapd() && current_may_throttle() &&
!sc->hibernation_mode &&
test_bit(LRUVEC_CONGESTED, &target_lruvec->flags))
- wait_iff_congested(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
if (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
sc))
@@ -4286,6 +4343,7 @@ static int kswapd(void *p)
WRITE_ONCE(pgdat->kswapd_order, 0);
WRITE_ONCE(pgdat->kswapd_highest_zoneidx, MAX_NR_ZONES);
+ atomic_set(&pgdat->nr_writeback_throttled, 0);
for ( ; ; ) {
bool ret;
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 8ce2620344b2..9b2bc9d61d4b 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1225,6 +1225,7 @@ const char * const vmstat_text[] = {
"nr_vmscan_immediate_reclaim",
"nr_dirtied",
"nr_written",
+ "nr_throttled_written",
"nr_kernel_misc_reclaimable",
"nr_foll_pin_acquired",
"nr_foll_pin_released",
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/8] mm/vmscan: Throttle reclaim and compaction when too may pages are isolated
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
2021-10-22 14:46 ` [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-22 14:46 ` [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made Mel Gorman
` (5 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Page reclaim throttles on congestion if too many parallel reclaim instances
have isolated too many pages. This makes no sense, excessive parallelisation
has nothing to do with writeback or congestion.
This patch creates an additional workqueue to sleep on when too many
pages are isolated. The throttled tasks are woken when the number
of isolated pages is reduced or a timeout occurs. There may be
some false positive wakeups for GFP_NOIO/GFP_NOFS callers but
the tasks will throttle again if necessary.
[shy828301@gmail.com: Wake up from compaction context]
[vbabka@suse.cz: Account number of throttled tasks only for writeback]
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/mmzone.h | 1 +
include/trace/events/vmscan.h | 4 +++-
mm/compaction.c | 10 ++++++++--
mm/internal.h | 11 +++++++++++
mm/vmscan.c | 22 ++++++++++++++++------
5 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 419304093610..9ccd8d95291b 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -275,6 +275,7 @@ enum lru_list {
enum vmscan_throttle_state {
VMSCAN_THROTTLE_WRITEBACK,
+ VMSCAN_THROTTLE_ISOLATED,
NR_VMSCAN_THROTTLE,
};
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index c317f9fe0d17..d4905bd9e9c4 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -28,10 +28,12 @@
) : "RECLAIM_WB_NONE"
#define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
+#define _VMSCAN_THROTTLE_ISOLATED (1 << VMSCAN_THROTTLE_ISOLATED)
#define show_throttle_flags(flags) \
(flags) ? __print_flags(flags, "|", \
- {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"} \
+ {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"}, \
+ {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"} \
) : "VMSCAN_THROTTLE_NONE"
diff --git a/mm/compaction.c b/mm/compaction.c
index bfc93da1c2c7..7359093d8ac0 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -761,6 +761,8 @@ isolate_freepages_range(struct compact_control *cc,
/* Similar to reclaim, but different enough that they don't share logic */
static bool too_many_isolated(pg_data_t *pgdat)
{
+ bool too_many;
+
unsigned long active, inactive, isolated;
inactive = node_page_state(pgdat, NR_INACTIVE_FILE) +
@@ -770,7 +772,11 @@ static bool too_many_isolated(pg_data_t *pgdat)
isolated = node_page_state(pgdat, NR_ISOLATED_FILE) +
node_page_state(pgdat, NR_ISOLATED_ANON);
- return isolated > (inactive + active) / 2;
+ too_many = isolated > (inactive + active) / 2;
+ if (!too_many)
+ wake_throttle_isolated(pgdat);
+
+ return too_many;
}
/**
@@ -822,7 +828,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
if (cc->mode == MIGRATE_ASYNC)
return -EAGAIN;
- congestion_wait(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED, HZ/10);
if (fatal_signal_pending(current))
return -EINTR;
diff --git a/mm/internal.h b/mm/internal.h
index b495e60c955d..c72d3383ef34 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -45,6 +45,15 @@ static inline void acct_reclaim_writeback(struct page *page)
__acct_reclaim_writeback(pgdat, page, nr_throttled);
}
+static inline void wake_throttle_isolated(pg_data_t *pgdat)
+{
+ wait_queue_head_t *wqh;
+
+ wqh = &pgdat->reclaim_wait[VMSCAN_THROTTLE_ISOLATED];
+ if (waitqueue_active(wqh))
+ wake_up(wqh);
+}
+
vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
@@ -120,6 +129,8 @@ extern unsigned long highest_memmap_pfn;
*/
extern int isolate_lru_page(struct page *page);
extern void putback_lru_page(struct page *page);
+extern void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
+ long timeout);
/*
* in mm/rmap.c:
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0c1595065f49..1e54e636b927 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1006,12 +1006,12 @@ static void handle_write_error(struct address_space *mapping,
unlock_page(page);
}
-static void
-reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
+void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
long timeout)
{
wait_queue_head_t *wqh = &pgdat->reclaim_wait[reason];
long ret;
+ bool acct_writeback = (reason == VMSCAN_THROTTLE_WRITEBACK);
DEFINE_WAIT(wait);
/*
@@ -1023,7 +1023,8 @@ reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
current->flags & (PF_IO_WORKER|PF_KTHREAD))
return;
- if (atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
+ if (acct_writeback &&
+ atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
WRITE_ONCE(pgdat->nr_reclaim_start,
node_page_state(pgdat, NR_THROTTLED_WRITTEN));
}
@@ -1031,7 +1032,9 @@ reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
ret = schedule_timeout(timeout);
finish_wait(wqh, &wait);
- atomic_dec(&pgdat->nr_writeback_throttled);
+
+ if (acct_writeback)
+ atomic_dec(&pgdat->nr_writeback_throttled);
trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
jiffies_to_usecs(timeout - ret),
@@ -2176,6 +2179,7 @@ static int too_many_isolated(struct pglist_data *pgdat, int file,
struct scan_control *sc)
{
unsigned long inactive, isolated;
+ bool too_many;
if (current_is_kswapd())
return 0;
@@ -2199,7 +2203,13 @@ static int too_many_isolated(struct pglist_data *pgdat, int file,
if ((sc->gfp_mask & (__GFP_IO | __GFP_FS)) == (__GFP_IO | __GFP_FS))
inactive >>= 3;
- return isolated > inactive;
+ too_many = isolated > inactive;
+
+ /* Wake up tasks throttled due to too_many_isolated. */
+ if (!too_many)
+ wake_throttle_isolated(pgdat);
+
+ return too_many;
}
/*
@@ -2308,8 +2318,8 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
return 0;
/* wait a bit for the reclaimer. */
- msleep(100);
stalled = true;
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED, HZ/10);
/* We are about to die and free our memory. Return now. */
if (fatal_signal_pending(current))
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
2021-10-22 14:46 ` [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested Mel Gorman
2021-10-22 14:46 ` [PATCH 2/8] mm/vmscan: Throttle reclaim and compaction when too may pages are isolated Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-11-24 1:19 ` Darrick J. Wong
2021-10-22 14:46 ` [PATCH 4/8] mm/writeback: Throttle based on page writeback instead of congestion Mel Gorman
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Memcg reclaim throttles on congestion if no reclaim progress is made.
This makes little sense, it might be due to writeback or a host of
other factors.
For !memcg reclaim, it's messy. Direct reclaim primarily is throttled
in the page allocator if it is failing to make progress. Kswapd
throttles if too many pages are under writeback and marked for
immediate reclaim.
This patch explicitly throttles if reclaim is failing to make progress.
[vbabka@suse.cz: Remove redundant code]
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/mmzone.h | 1 +
include/trace/events/vmscan.h | 4 +++-
mm/memcontrol.c | 10 +---------
mm/vmscan.c | 28 ++++++++++++++++++++++++++++
4 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9ccd8d95291b..00e305cfb3ec 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -276,6 +276,7 @@ enum lru_list {
enum vmscan_throttle_state {
VMSCAN_THROTTLE_WRITEBACK,
VMSCAN_THROTTLE_ISOLATED,
+ VMSCAN_THROTTLE_NOPROGRESS,
NR_VMSCAN_THROTTLE,
};
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index d4905bd9e9c4..f25a6149d3ba 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -29,11 +29,13 @@
#define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
#define _VMSCAN_THROTTLE_ISOLATED (1 << VMSCAN_THROTTLE_ISOLATED)
+#define _VMSCAN_THROTTLE_NOPROGRESS (1 << VMSCAN_THROTTLE_NOPROGRESS)
#define show_throttle_flags(flags) \
(flags) ? __print_flags(flags, "|", \
{_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"}, \
- {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"} \
+ {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"}, \
+ {_VMSCAN_THROTTLE_NOPROGRESS, "VMSCAN_THROTTLE_NOPROGRESS"} \
) : "VMSCAN_THROTTLE_NONE"
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6da5020a8656..8b33152c9b85 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3465,19 +3465,11 @@ static int mem_cgroup_force_empty(struct mem_cgroup *memcg)
/* try to free all pages in this cgroup */
while (nr_retries && page_counter_read(&memcg->memory)) {
- int progress;
-
if (signal_pending(current))
return -EINTR;
- progress = try_to_free_mem_cgroup_pages(memcg, 1,
- GFP_KERNEL, true);
- if (!progress) {
+ if (!try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true))
nr_retries--;
- /* maybe some writeback is necessary */
- congestion_wait(BLK_RW_ASYNC, HZ/10);
- }
-
}
return 0;
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1e54e636b927..0450f6867d61 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3323,6 +3323,33 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
return zone_watermark_ok_safe(zone, 0, watermark, sc->reclaim_idx);
}
+static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
+{
+ /* If reclaim is making progress, wake any throttled tasks. */
+ if (sc->nr_reclaimed) {
+ wait_queue_head_t *wqh;
+
+ wqh = &pgdat->reclaim_wait[VMSCAN_THROTTLE_NOPROGRESS];
+ if (waitqueue_active(wqh))
+ wake_up(wqh);
+
+ return;
+ }
+
+ /*
+ * Do not throttle kswapd on NOPROGRESS as it will throttle on
+ * VMSCAN_THROTTLE_WRITEBACK if there are too many pages under
+ * writeback and marked for immediate reclaim at the tail of
+ * the LRU.
+ */
+ if (current_is_kswapd())
+ return;
+
+ /* Throttle if making no progress at high prioities. */
+ if (sc->priority < DEF_PRIORITY - 2)
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS, HZ/10);
+}
+
/*
* This is the direct reclaim path, for page-allocating processes. We only
* try to reclaim pages from zones which will satisfy the caller's allocation
@@ -3407,6 +3434,7 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
continue;
last_pgdat = zone->zone_pgdat;
shrink_node(zone->zone_pgdat, sc);
+ consider_reclaim_throttle(zone->zone_pgdat, sc);
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-10-22 14:46 ` [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made Mel Gorman
@ 2021-11-24 1:19 ` Darrick J. Wong
2021-11-24 1:49 ` Darrick J. Wong
2021-11-24 10:32 ` Mel Gorman
0 siblings, 2 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-11-24 1:19 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Fri, Oct 22, 2021 at 03:46:46PM +0100, Mel Gorman wrote:
> Memcg reclaim throttles on congestion if no reclaim progress is made.
> This makes little sense, it might be due to writeback or a host of
> other factors.
>
> For !memcg reclaim, it's messy. Direct reclaim primarily is throttled
> in the page allocator if it is failing to make progress. Kswapd
> throttles if too many pages are under writeback and marked for
> immediate reclaim.
>
> This patch explicitly throttles if reclaim is failing to make progress.
Hi Mel,
Ever since Christoph broke swapfiles, I've been carrying around a little
fstest in my dev tree[1] that tries to exercise paging things in and out
of a swapfile. Sadly I've been trapped in about three dozen customer
escalations for over a month, which means I haven't been able to do much
upstream in weeks. Like submit this test upstream. :(
Now that I've finally gotten around to trying out a 5.16-rc2 build, I
notice that the runtime of this test has gone from ~5s to 2 hours.
Among other things that it does, the test sets up a cgroup with a memory
controller limiting the memory usage to 25MB, then runs a program that
tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
we're not running reclaim globally, but the cgroup gets throttled very
severely.
AFAICT the system is mostly idle, but it's difficult to tell because ps
and top also get stuck waiting for this cgroup for whatever reason. My
uninformed spculation is that usemem_and_swapoff takes a page fault
while dirtying the 50MB memory buffer, prepares to pull a page in from
swap, tries to evict another page to stay under the memcg limit, but
that decides that it's making no progress and calls
reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
down. Eventually we either finish the test or (for the mlock part) the
OOM killer actually kills the process, but this takes a very long time.
Any thoughts? For now I can just hack around this by skipping
reclaim_throttle if cgroup_reclaim() == true, but that's probably not
the correct fix. :)
--D
[1] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/commit/?h=test-swapfile-io&id=0d0ad843cea366d0ab0a7d8d984e5cd1deba5b43
>
> [vbabka@suse.cz: Remove redundant code]
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> include/linux/mmzone.h | 1 +
> include/trace/events/vmscan.h | 4 +++-
> mm/memcontrol.c | 10 +---------
> mm/vmscan.c | 28 ++++++++++++++++++++++++++++
> 4 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 9ccd8d95291b..00e305cfb3ec 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -276,6 +276,7 @@ enum lru_list {
> enum vmscan_throttle_state {
> VMSCAN_THROTTLE_WRITEBACK,
> VMSCAN_THROTTLE_ISOLATED,
> + VMSCAN_THROTTLE_NOPROGRESS,
> NR_VMSCAN_THROTTLE,
> };
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index d4905bd9e9c4..f25a6149d3ba 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -29,11 +29,13 @@
>
> #define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
> #define _VMSCAN_THROTTLE_ISOLATED (1 << VMSCAN_THROTTLE_ISOLATED)
> +#define _VMSCAN_THROTTLE_NOPROGRESS (1 << VMSCAN_THROTTLE_NOPROGRESS)
>
> #define show_throttle_flags(flags) \
> (flags) ? __print_flags(flags, "|", \
> {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"}, \
> - {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"} \
> + {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"}, \
> + {_VMSCAN_THROTTLE_NOPROGRESS, "VMSCAN_THROTTLE_NOPROGRESS"} \
> ) : "VMSCAN_THROTTLE_NONE"
>
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6da5020a8656..8b33152c9b85 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3465,19 +3465,11 @@ static int mem_cgroup_force_empty(struct mem_cgroup *memcg)
>
> /* try to free all pages in this cgroup */
> while (nr_retries && page_counter_read(&memcg->memory)) {
> - int progress;
> -
> if (signal_pending(current))
> return -EINTR;
>
> - progress = try_to_free_mem_cgroup_pages(memcg, 1,
> - GFP_KERNEL, true);
> - if (!progress) {
> + if (!try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true))
> nr_retries--;
> - /* maybe some writeback is necessary */
> - congestion_wait(BLK_RW_ASYNC, HZ/10);
> - }
> -
> }
>
> return 0;
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 1e54e636b927..0450f6867d61 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3323,6 +3323,33 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> return zone_watermark_ok_safe(zone, 0, watermark, sc->reclaim_idx);
> }
>
> +static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
> +{
> + /* If reclaim is making progress, wake any throttled tasks. */
> + if (sc->nr_reclaimed) {
> + wait_queue_head_t *wqh;
> +
> + wqh = &pgdat->reclaim_wait[VMSCAN_THROTTLE_NOPROGRESS];
> + if (waitqueue_active(wqh))
> + wake_up(wqh);
> +
> + return;
> + }
> +
> + /*
> + * Do not throttle kswapd on NOPROGRESS as it will throttle on
> + * VMSCAN_THROTTLE_WRITEBACK if there are too many pages under
> + * writeback and marked for immediate reclaim at the tail of
> + * the LRU.
> + */
> + if (current_is_kswapd())
> + return;
> +
> + /* Throttle if making no progress at high prioities. */
> + if (sc->priority < DEF_PRIORITY - 2)
> + reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS, HZ/10);
> +}
> +
> /*
> * This is the direct reclaim path, for page-allocating processes. We only
> * try to reclaim pages from zones which will satisfy the caller's allocation
> @@ -3407,6 +3434,7 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> continue;
> last_pgdat = zone->zone_pgdat;
> shrink_node(zone->zone_pgdat, sc);
> + consider_reclaim_throttle(zone->zone_pgdat, sc);
> }
>
> /*
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 1:19 ` Darrick J. Wong
@ 2021-11-24 1:49 ` Darrick J. Wong
2021-11-24 14:35 ` Mel Gorman
2021-11-24 10:32 ` Mel Gorman
1 sibling, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2021-11-24 1:49 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Tue, Nov 23, 2021 at 05:19:12PM -0800, Darrick J. Wong wrote:
> On Fri, Oct 22, 2021 at 03:46:46PM +0100, Mel Gorman wrote:
> > Memcg reclaim throttles on congestion if no reclaim progress is made.
> > This makes little sense, it might be due to writeback or a host of
> > other factors.
> >
> > For !memcg reclaim, it's messy. Direct reclaim primarily is throttled
> > in the page allocator if it is failing to make progress. Kswapd
> > throttles if too many pages are under writeback and marked for
> > immediate reclaim.
> >
> > This patch explicitly throttles if reclaim is failing to make progress.
>
> Hi Mel,
>
> Ever since Christoph broke swapfiles, I've been carrying around a little
> fstest in my dev tree[1] that tries to exercise paging things in and out
> of a swapfile. Sadly I've been trapped in about three dozen customer
> escalations for over a month, which means I haven't been able to do much
> upstream in weeks. Like submit this test upstream. :(
>
> Now that I've finally gotten around to trying out a 5.16-rc2 build, I
> notice that the runtime of this test has gone from ~5s to 2 hours.
> Among other things that it does, the test sets up a cgroup with a memory
> controller limiting the memory usage to 25MB, then runs a program that
> tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
> we're not running reclaim globally, but the cgroup gets throttled very
> severely.
>
> AFAICT the system is mostly idle, but it's difficult to tell because ps
> and top also get stuck waiting for this cgroup for whatever reason. My
> uninformed spculation is that usemem_and_swapoff takes a page fault
> while dirtying the 50MB memory buffer, prepares to pull a page in from
> swap, tries to evict another page to stay under the memcg limit, but
> that decides that it's making no progress and calls
> reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
>
> The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
> down. Eventually we either finish the test or (for the mlock part) the
> OOM killer actually kills the process, but this takes a very long time.
>
> Any thoughts? For now I can just hack around this by skipping
> reclaim_throttle if cgroup_reclaim() == true, but that's probably not
> the correct fix. :)
Update: after adding timing information to usemem_and_swapoff, it looks
like dirtying the 50MB buffer takes ~22s (up from 0.06s on 5.15). The
mlock call stalls for ~280s until the OOM killer kills it (up from
nearly instantaneous on 5.15), and the swapon/swapoff variant takes
20 minutes to hours depending on the run.
--D
> --D
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/commit/?h=test-swapfile-io&id=0d0ad843cea366d0ab0a7d8d984e5cd1deba5b43
>
> >
> > [vbabka@suse.cz: Remove redundant code]
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > ---
> > include/linux/mmzone.h | 1 +
> > include/trace/events/vmscan.h | 4 +++-
> > mm/memcontrol.c | 10 +---------
> > mm/vmscan.c | 28 ++++++++++++++++++++++++++++
> > 4 files changed, 33 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index 9ccd8d95291b..00e305cfb3ec 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -276,6 +276,7 @@ enum lru_list {
> > enum vmscan_throttle_state {
> > VMSCAN_THROTTLE_WRITEBACK,
> > VMSCAN_THROTTLE_ISOLATED,
> > + VMSCAN_THROTTLE_NOPROGRESS,
> > NR_VMSCAN_THROTTLE,
> > };
> >
> > diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> > index d4905bd9e9c4..f25a6149d3ba 100644
> > --- a/include/trace/events/vmscan.h
> > +++ b/include/trace/events/vmscan.h
> > @@ -29,11 +29,13 @@
> >
> > #define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
> > #define _VMSCAN_THROTTLE_ISOLATED (1 << VMSCAN_THROTTLE_ISOLATED)
> > +#define _VMSCAN_THROTTLE_NOPROGRESS (1 << VMSCAN_THROTTLE_NOPROGRESS)
> >
> > #define show_throttle_flags(flags) \
> > (flags) ? __print_flags(flags, "|", \
> > {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"}, \
> > - {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"} \
> > + {_VMSCAN_THROTTLE_ISOLATED, "VMSCAN_THROTTLE_ISOLATED"}, \
> > + {_VMSCAN_THROTTLE_NOPROGRESS, "VMSCAN_THROTTLE_NOPROGRESS"} \
> > ) : "VMSCAN_THROTTLE_NONE"
> >
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 6da5020a8656..8b33152c9b85 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -3465,19 +3465,11 @@ static int mem_cgroup_force_empty(struct mem_cgroup *memcg)
> >
> > /* try to free all pages in this cgroup */
> > while (nr_retries && page_counter_read(&memcg->memory)) {
> > - int progress;
> > -
> > if (signal_pending(current))
> > return -EINTR;
> >
> > - progress = try_to_free_mem_cgroup_pages(memcg, 1,
> > - GFP_KERNEL, true);
> > - if (!progress) {
> > + if (!try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true))
> > nr_retries--;
> > - /* maybe some writeback is necessary */
> > - congestion_wait(BLK_RW_ASYNC, HZ/10);
> > - }
> > -
> > }
> >
> > return 0;
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 1e54e636b927..0450f6867d61 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3323,6 +3323,33 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > return zone_watermark_ok_safe(zone, 0, watermark, sc->reclaim_idx);
> > }
> >
> > +static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
> > +{
> > + /* If reclaim is making progress, wake any throttled tasks. */
> > + if (sc->nr_reclaimed) {
> > + wait_queue_head_t *wqh;
> > +
> > + wqh = &pgdat->reclaim_wait[VMSCAN_THROTTLE_NOPROGRESS];
> > + if (waitqueue_active(wqh))
> > + wake_up(wqh);
> > +
> > + return;
> > + }
> > +
> > + /*
> > + * Do not throttle kswapd on NOPROGRESS as it will throttle on
> > + * VMSCAN_THROTTLE_WRITEBACK if there are too many pages under
> > + * writeback and marked for immediate reclaim at the tail of
> > + * the LRU.
> > + */
> > + if (current_is_kswapd())
> > + return;
> > +
> > + /* Throttle if making no progress at high prioities. */
> > + if (sc->priority < DEF_PRIORITY - 2)
> > + reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS, HZ/10);
> > +}
> > +
> > /*
> > * This is the direct reclaim path, for page-allocating processes. We only
> > * try to reclaim pages from zones which will satisfy the caller's allocation
> > @@ -3407,6 +3434,7 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> > continue;
> > last_pgdat = zone->zone_pgdat;
> > shrink_node(zone->zone_pgdat, sc);
> > + consider_reclaim_throttle(zone->zone_pgdat, sc);
> > }
> >
> > /*
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 1:49 ` Darrick J. Wong
@ 2021-11-24 14:35 ` Mel Gorman
2021-11-24 18:02 ` Darrick J. Wong
0 siblings, 1 reply; 23+ messages in thread
From: Mel Gorman @ 2021-11-24 14:35 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Tue, Nov 23, 2021 at 05:49:14PM -0800, Darrick J. Wong wrote:
> > Ever since Christoph broke swapfiles, I've been carrying around a little
> > fstest in my dev tree[1] that tries to exercise paging things in and out
> > of a swapfile. Sadly I've been trapped in about three dozen customer
> > escalations for over a month, which means I haven't been able to do much
> > upstream in weeks. Like submit this test upstream. :(
> >
> > Now that I've finally gotten around to trying out a 5.16-rc2 build, I
> > notice that the runtime of this test has gone from ~5s to 2 hours.
> > Among other things that it does, the test sets up a cgroup with a memory
> > controller limiting the memory usage to 25MB, then runs a program that
> > tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
> > we're not running reclaim globally, but the cgroup gets throttled very
> > severely.
> >
> > AFAICT the system is mostly idle, but it's difficult to tell because ps
> > and top also get stuck waiting for this cgroup for whatever reason. My
> > uninformed spculation is that usemem_and_swapoff takes a page fault
> > while dirtying the 50MB memory buffer, prepares to pull a page in from
> > swap, tries to evict another page to stay under the memcg limit, but
> > that decides that it's making no progress and calls
> > reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
> >
> > The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
> > down. Eventually we either finish the test or (for the mlock part) the
> > OOM killer actually kills the process, but this takes a very long time.
> >
> > Any thoughts? For now I can just hack around this by skipping
> > reclaim_throttle if cgroup_reclaim() == true, but that's probably not
> > the correct fix. :)
>
> Update: after adding timing information to usemem_and_swapoff, it looks
> like dirtying the 50MB buffer takes ~22s (up from 0.06s on 5.15). The
> mlock call stalls for ~280s until the OOM killer kills it (up from
> nearly instantaneous on 5.15), and the swapon/swapoff variant takes
> 20 minutes to hours depending on the run.
>
Can you try the patch below please? I think I'm running the test
correctly and it finishes for me in 16 seconds with this applied
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 07db03883062..d9166e94eb95 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1057,7 +1057,17 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
break;
case VMSCAN_THROTTLE_NOPROGRESS:
- timeout = HZ/2;
+ timeout = 1;
+
+ /*
+ * If kswapd is disabled, reschedule if necessary but do not
+ * throttle as the system is likely near OOM.
+ */
+ if (pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES) {
+ cond_resched();
+ return;
+ }
+
break;
case VMSCAN_THROTTLE_ISOLATED:
timeout = HZ/50;
@@ -3395,7 +3405,7 @@ static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
return;
/* Throttle if making no progress at high prioities. */
- if (sc->priority < DEF_PRIORITY - 2)
+ if (sc->priority < DEF_PRIORITY - 2 && !sc->nr_reclaimed)
reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS);
}
@@ -3415,6 +3425,7 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
unsigned long nr_soft_scanned;
gfp_t orig_mask;
pg_data_t *last_pgdat = NULL;
+ pg_data_t *first_pgdat = NULL;
/*
* If the number of buffer_heads in the machine exceeds the maximum
@@ -3478,14 +3489,18 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
/* need some check for avoid more shrink_zone() */
}
+ if (!first_pgdat)
+ first_pgdat = zone->zone_pgdat;
+
/* See comment about same check for global reclaim above */
if (zone->zone_pgdat == last_pgdat)
continue;
last_pgdat = zone->zone_pgdat;
shrink_node(zone->zone_pgdat, sc);
- consider_reclaim_throttle(zone->zone_pgdat, sc);
}
+ consider_reclaim_throttle(first_pgdat, sc);
+
/*
* Restore to original mask to avoid the impact on the caller if we
* promoted it to __GFP_HIGHMEM.
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 14:35 ` Mel Gorman
@ 2021-11-24 18:02 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-11-24 18:02 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Wed, Nov 24, 2021 at 02:35:59PM +0000, Mel Gorman wrote:
> On Tue, Nov 23, 2021 at 05:49:14PM -0800, Darrick J. Wong wrote:
> > > Ever since Christoph broke swapfiles, I've been carrying around a little
> > > fstest in my dev tree[1] that tries to exercise paging things in and out
> > > of a swapfile. Sadly I've been trapped in about three dozen customer
> > > escalations for over a month, which means I haven't been able to do much
> > > upstream in weeks. Like submit this test upstream. :(
> > >
> > > Now that I've finally gotten around to trying out a 5.16-rc2 build, I
> > > notice that the runtime of this test has gone from ~5s to 2 hours.
> > > Among other things that it does, the test sets up a cgroup with a memory
> > > controller limiting the memory usage to 25MB, then runs a program that
> > > tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
> > > we're not running reclaim globally, but the cgroup gets throttled very
> > > severely.
> > >
> > > AFAICT the system is mostly idle, but it's difficult to tell because ps
> > > and top also get stuck waiting for this cgroup for whatever reason. My
> > > uninformed spculation is that usemem_and_swapoff takes a page fault
> > > while dirtying the 50MB memory buffer, prepares to pull a page in from
> > > swap, tries to evict another page to stay under the memcg limit, but
> > > that decides that it's making no progress and calls
> > > reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
> > >
> > > The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
> > > down. Eventually we either finish the test or (for the mlock part) the
> > > OOM killer actually kills the process, but this takes a very long time.
> > >
> > > Any thoughts? For now I can just hack around this by skipping
> > > reclaim_throttle if cgroup_reclaim() == true, but that's probably not
> > > the correct fix. :)
> >
> > Update: after adding timing information to usemem_and_swapoff, it looks
> > like dirtying the 50MB buffer takes ~22s (up from 0.06s on 5.15). The
> > mlock call stalls for ~280s until the OOM killer kills it (up from
> > nearly instantaneous on 5.15), and the swapon/swapoff variant takes
> > 20 minutes to hours depending on the run.
> >
>
> Can you try the patch below please? I think I'm running the test
> correctly and it finishes for me in 16 seconds with this applied
20 seconds here, but this /does/ fix the problem. Thank you!
Tested-by: Darrick J. Wong <djwong@kernel.org>
--D
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 07db03883062..d9166e94eb95 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1057,7 +1057,17 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
>
> break;
> case VMSCAN_THROTTLE_NOPROGRESS:
> - timeout = HZ/2;
> + timeout = 1;
> +
> + /*
> + * If kswapd is disabled, reschedule if necessary but do not
> + * throttle as the system is likely near OOM.
> + */
> + if (pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES) {
> + cond_resched();
> + return;
> + }
> +
> break;
> case VMSCAN_THROTTLE_ISOLATED:
> timeout = HZ/50;
> @@ -3395,7 +3405,7 @@ static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
> return;
>
> /* Throttle if making no progress at high prioities. */
> - if (sc->priority < DEF_PRIORITY - 2)
> + if (sc->priority < DEF_PRIORITY - 2 && !sc->nr_reclaimed)
> reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS);
> }
>
> @@ -3415,6 +3425,7 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> unsigned long nr_soft_scanned;
> gfp_t orig_mask;
> pg_data_t *last_pgdat = NULL;
> + pg_data_t *first_pgdat = NULL;
>
> /*
> * If the number of buffer_heads in the machine exceeds the maximum
> @@ -3478,14 +3489,18 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> /* need some check for avoid more shrink_zone() */
> }
>
> + if (!first_pgdat)
> + first_pgdat = zone->zone_pgdat;
> +
> /* See comment about same check for global reclaim above */
> if (zone->zone_pgdat == last_pgdat)
> continue;
> last_pgdat = zone->zone_pgdat;
> shrink_node(zone->zone_pgdat, sc);
> - consider_reclaim_throttle(zone->zone_pgdat, sc);
> }
>
> + consider_reclaim_throttle(first_pgdat, sc);
> +
> /*
> * Restore to original mask to avoid the impact on the caller if we
> * promoted it to __GFP_HIGHMEM.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 1:19 ` Darrick J. Wong
2021-11-24 1:49 ` Darrick J. Wong
@ 2021-11-24 10:32 ` Mel Gorman
2021-11-24 10:43 ` Vlastimil Babka
2021-11-24 17:24 ` Mike Galbraith
1 sibling, 2 replies; 23+ messages in thread
From: Mel Gorman @ 2021-11-24 10:32 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Tue, Nov 23, 2021 at 05:19:12PM -0800, Darrick J. Wong wrote:
> On Fri, Oct 22, 2021 at 03:46:46PM +0100, Mel Gorman wrote:
> > Memcg reclaim throttles on congestion if no reclaim progress is made.
> > This makes little sense, it might be due to writeback or a host of
> > other factors.
> >
> > For !memcg reclaim, it's messy. Direct reclaim primarily is throttled
> > in the page allocator if it is failing to make progress. Kswapd
> > throttles if too many pages are under writeback and marked for
> > immediate reclaim.
> >
> > This patch explicitly throttles if reclaim is failing to make progress.
>
> Hi Mel,
>
> Ever since Christoph broke swapfiles, I've been carrying around a little
> fstest in my dev tree[1] that tries to exercise paging things in and out
> of a swapfile. Sadly I've been trapped in about three dozen customer
> escalations for over a month, which means I haven't been able to do much
> upstream in weeks. Like submit this test upstream. :(
>
> Now that I've finally gotten around to trying out a 5.16-rc2 build, I
> notice that the runtime of this test has gone from ~5s to 2 hours.
> Among other things that it does, the test sets up a cgroup with a memory
> controller limiting the memory usage to 25MB, then runs a program that
> tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
> we're not running reclaim globally, but the cgroup gets throttled very
> severely.
>
Ok, so this test cannot make progress until some of the cgroup pages get
cleaned. What is the expectation for the test? Should it OOM or do you
expect it to have spin-like behaviour until some writeback completes?
I'm guessing you'd prefer it to spin and right now it's sleeping far
too much.
> AFAICT the system is mostly idle, but it's difficult to tell because ps
> and top also get stuck waiting for this cgroup for whatever reason.
But this is surprising because I expect that ps and top are not running
within the cgroup. Was /proc/PID/stack readable?
> My
> uninformed spculation is that usemem_and_swapoff takes a page fault
> while dirtying the 50MB memory buffer, prepares to pull a page in from
> swap, tries to evict another page to stay under the memcg limit, but
> that decides that it's making no progress and calls
> reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
>
> The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
> down. Eventually we either finish the test or (for the mlock part) the
> OOM killer actually kills the process, but this takes a very long time.
>
The sleep can be interruptible.
> Any thoughts? For now I can just hack around this by skipping
> reclaim_throttle if cgroup_reclaim() == true, but that's probably not
> the correct fix. :)
>
No, it wouldn't be but a possibility is throttling for only 1 jiffy if
reclaiming within a memcg and the zone is balanced overall.
The interruptible part should just be the patch below. I need to poke at
the cgroup limit part a bit
diff --git a/mm/vmscan.c b/mm/vmscan.c
index fb9584641ac7..07db03883062 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1068,7 +1068,7 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
break;
}
- prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+ prepare_to_wait(wqh, &wait, TASK_INTERRUPTIBLE);
ret = schedule_timeout(timeout);
finish_wait(wqh, &wait);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 10:32 ` Mel Gorman
@ 2021-11-24 10:43 ` Vlastimil Babka
2021-11-24 10:53 ` Mel Gorman
2021-11-24 17:24 ` Mike Galbraith
1 sibling, 1 reply; 23+ messages in thread
From: Vlastimil Babka @ 2021-11-24 10:43 UTC (permalink / raw)
To: Mel Gorman, Darrick J. Wong
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Johannes Weiner, Jonathan Corbet, Linux-MM, Linux-fsdevel, LKML
On 11/24/21 11:32, Mel Gorman wrote:
> On Tue, Nov 23, 2021 at 05:19:12PM -0800, Darrick J. Wong wrote:
>> On Fri, Oct 22, 2021 at 03:46:46PM +0100, Mel Gorman wrote:
>> > Memcg reclaim throttles on congestion if no reclaim progress is made.
>> > This makes little sense, it might be due to writeback or a host of
>> > other factors.
>> >
>> > For !memcg reclaim, it's messy. Direct reclaim primarily is throttled
>> > in the page allocator if it is failing to make progress. Kswapd
>> > throttles if too many pages are under writeback and marked for
>> > immediate reclaim.
>> >
>> > This patch explicitly throttles if reclaim is failing to make progress.
>>
>> Hi Mel,
>>
>> Ever since Christoph broke swapfiles, I've been carrying around a little
>> fstest in my dev tree[1] that tries to exercise paging things in and out
>> of a swapfile. Sadly I've been trapped in about three dozen customer
>> escalations for over a month, which means I haven't been able to do much
>> upstream in weeks. Like submit this test upstream. :(
>>
>> Now that I've finally gotten around to trying out a 5.16-rc2 build, I
>> notice that the runtime of this test has gone from ~5s to 2 hours.
>> Among other things that it does, the test sets up a cgroup with a memory
>> controller limiting the memory usage to 25MB, then runs a program that
>> tries to dirty 50MB of memory. There's 2GB of memory in the VM, so
>> we're not running reclaim globally, but the cgroup gets throttled very
>> severely.
>>
>
> Ok, so this test cannot make progress until some of the cgroup pages get
> cleaned. What is the expectation for the test? Should it OOM or do you
> expect it to have spin-like behaviour until some writeback completes?
> I'm guessing you'd prefer it to spin and right now it's sleeping far
> too much.
>
>> AFAICT the system is mostly idle, but it's difficult to tell because ps
>> and top also get stuck waiting for this cgroup for whatever reason.
>
> But this is surprising because I expect that ps and top are not running
> within the cgroup. Was /proc/PID/stack readable?
>
>> My
>> uninformed spculation is that usemem_and_swapoff takes a page fault
>> while dirtying the 50MB memory buffer, prepares to pull a page in from
>> swap, tries to evict another page to stay under the memcg limit, but
>> that decides that it's making no progress and calls
>> reclaim_throttle(..., VMSCAN_THROTTLE_NOPROGRESS).
>>
>> The sleep is uninterruptible, so I can't even kill -9 fstests to shut it
>> down. Eventually we either finish the test or (for the mlock part) the
>> OOM killer actually kills the process, but this takes a very long time.
>>
>
> The sleep can be interruptible.
>
>> Any thoughts? For now I can just hack around this by skipping
>> reclaim_throttle if cgroup_reclaim() == true, but that's probably not
>> the correct fix. :)
>>
>
> No, it wouldn't be but a possibility is throttling for only 1 jiffy if
> reclaiming within a memcg and the zone is balanced overall.
>
> The interruptible part should just be the patch below. I need to poke at
> the cgroup limit part a bit
As the throttle timeout is short anyway, will the TASK_UNINTERRUPTIBLE vs
TASK_INTERRUPTIBLE make a difference for the (ability to kill? AFAIU
typically this inability to kill is because of a loop that doesn't check for
fatal_signal_pending().
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index fb9584641ac7..07db03883062 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1068,7 +1068,7 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
> break;
> }
>
> - prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
> + prepare_to_wait(wqh, &wait, TASK_INTERRUPTIBLE);
> ret = schedule_timeout(timeout);
> finish_wait(wqh, &wait);
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 10:43 ` Vlastimil Babka
@ 2021-11-24 10:53 ` Mel Gorman
0 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-11-24 10:53 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Darrick J. Wong, Andrew Morton, NeilBrown, Theodore Ts'o,
Andreas Dilger, Matthew Wilcox, Michal Hocko, Dave Chinner,
Rik van Riel, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Wed, Nov 24, 2021 at 11:43:05AM +0100, Vlastimil Babka wrote:
> >> Any thoughts? For now I can just hack around this by skipping
> >> reclaim_throttle if cgroup_reclaim() == true, but that's probably not
> >> the correct fix. :)
> >>
> >
> > No, it wouldn't be but a possibility is throttling for only 1 jiffy if
> > reclaiming within a memcg and the zone is balanced overall.
> >
> > The interruptible part should just be the patch below. I need to poke at
> > the cgroup limit part a bit
>
> As the throttle timeout is short anyway, will the TASK_UNINTERRUPTIBLE vs
> TASK_INTERRUPTIBLE make a difference for the (ability to kill? AFAIU
> typically this inability to kill is because of a loop that doesn't check for
> fatal_signal_pending().
>
Yep, and the fatal_signal_pending() is lacking within reclaim in general
but I'm undecided on how much that should change in the context of reclaim
throttling but at minimum, I don't want the signal delivery to be masked
or delayed.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made
2021-11-24 10:32 ` Mel Gorman
2021-11-24 10:43 ` Vlastimil Babka
@ 2021-11-24 17:24 ` Mike Galbraith
1 sibling, 0 replies; 23+ messages in thread
From: Mike Galbraith @ 2021-11-24 17:24 UTC (permalink / raw)
To: Mel Gorman, Darrick J. Wong
Cc: Andrew Morton, NeilBrown, Theodore Ts'o, Andreas Dilger,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML
On Wed, 2021-11-24 at 10:32 +0000, Mel Gorman wrote:
> On Tue, Nov 23, 2021 at 05:19:12PM -0800, Darrick J. Wong wrote:
>
> > AFAICT the system is mostly idle, but it's difficult to tell because ps
> > and top also get stuck waiting for this cgroup for whatever reason.
>
> But this is surprising because I expect that ps and top are not running
> within the cgroup. Was /proc/PID/stack readable?
Probably this.
crash> ps | grep UN
4418 4417 4 ffff8881cae66e40 UN 0.0 7620 980 memcg_test_1 <== the bad guy
4419 4417 6 ffff8881cae62f40 UN 0.0 7620 980 memcg_test_1
4420 4417 5 ffff8881cae65e80 UN 0.0 7620 980 memcg_test_1
4421 4417 7 ffff8881cae63f00 UN 0.0 7620 980 memcg_test_1
4422 4417 4 ffff8881cae60000 UN 0.0 7620 980 memcg_test_1
4423 4417 3 ffff888128985e80 UN 0.0 7620 980 memcg_test_1
4424 4417 7 ffff888117f79f80 UN 0.0 7620 980 memcg_test_1
4425 4417 2 ffff888117f7af40 UN 0.0 7620 980 memcg_test_1
4428 2791 6 ffff8881a8253f00 UN 0.0 38868 3568 ps
4429 2808 4 ffff888100c90000 UN 0.0 38868 3600 ps
crash> bt -sx 4429
PID: 4429 TASK: ffff888100c90000 CPU: 4 COMMAND: "ps"
#0 [ffff8881af1c3ce0] __schedule+0x285 at ffffffff817ae6c5
#1 [ffff8881af1c3d68] schedule+0x3a at ffffffff817aed4a
#2 [ffff8881af1c3d78] rwsem_down_read_slowpath+0x197 at ffffffff817b11a7
#3 [ffff8881af1c3e08] down_read_killable+0x5c at ffffffff817b142c
#4 [ffff8881af1c3e18] down_read_killable+0x5c at ffffffff817b142c
#5 [ffff8881af1c3e28] __access_remote_vm+0x3f at ffffffff8120131f
#6 [ffff8881af1c3e90] proc_pid_cmdline_read+0x148 at ffffffff812fc9a8
#7 [ffff8881af1c3ee8] vfs_read+0x92 at ffffffff8126a302
#8 [ffff8881af1c3f00] ksys_read+0x7d at ffffffff8126a72d
#9 [ffff8881af1c3f38] do_syscall_64+0x37 at ffffffff817a3f57
#10 [ffff8881af1c3f50] entry_SYSCALL_64_after_hwframe+0x44 at ffffffff8180007c
RIP: 00007f4b50fe8b5e RSP: 00007ffdd7f6fe38 RFLAGS: 00000246
RAX: ffffffffffffffda RBX: 00007f4b5186a010 RCX: 00007f4b50fe8b5e
RDX: 0000000000020000 RSI: 00007f4b5186a010 RDI: 0000000000000006
RBP: 0000000000020000 R8: 0000000000000007 R9: 00000000ffffffff
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f4b5186a010
R13: 0000000000000000 R14: 0000000000000006 R15: 0000000000000000
ORIG_RAX: 0000000000000000 CS: 0033 SS: 002b
crash> mm_struct -x ffff8881021b4800
struct mm_struct {
{
mmap = 0xffff8881ccfe6a80,
mm_rb = {
rb_node = 0xffff8881ccfe61a0
},
...
mmap_lock = {
count = {
counter = 0x3
},
owner = {
counter = 0xffff8881cae66e40
...
crash> bt 0xffff8881cae66e40
PID: 4418 TASK: ffff8881cae66e40 CPU: 4 COMMAND: "memcg_test_1"
#0 [ffff888154097a88] __schedule at ffffffff817ae6c5
#1 [ffff888154097b10] schedule at ffffffff817aed4a
#2 [ffff888154097b20] schedule_timeout at ffffffff817b311f
#3 [ffff888154097b90] reclaim_throttle at ffffffff811d802b
#4 [ffff888154097bf0] do_try_to_free_pages at ffffffff811da206
#5 [ffff888154097c40] try_to_free_mem_cgroup_pages at ffffffff811db522
#6 [ffff888154097cd0] try_charge_memcg at ffffffff81256440
#7 [ffff888154097d60] obj_cgroup_charge_pages at ffffffff81256c97
#8 [ffff888154097d88] obj_cgroup_charge at ffffffff8125898c
#9 [ffff888154097da8] kmem_cache_alloc at ffffffff81242099
#10 [ffff888154097de0] vm_area_alloc at ffffffff8106c87a
#11 [ffff888154097df0] mmap_region at ffffffff812082b2
#12 [ffff888154097e58] do_mmap at ffffffff81208922
#13 [ffff888154097eb0] vm_mmap_pgoff at ffffffff811e259f
#14 [ffff888154097f38] do_syscall_64 at ffffffff817a3f57
#15 [ffff888154097f50] entry_SYSCALL_64_after_hwframe at
ffffffff8180007c
RIP: 00007f211c36b743 RSP: 00007ffeaac1bd58 RFLAGS: 00000246
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f211c36b743
RDX: 0000000000000003 RSI: 0000000000001000 RDI: 0000000000000000
RBP: 0000000000000000 R8: 0000000000000000 R9: 0000000000000000
R10: 0000000000002022 R11: 0000000000000246 R12: 0000000000000003
R13: 0000000000001000 R14: 0000000000002022 R15: 0000000000000000
ORIG_RAX: 0000000000000009 CS: 0033 SS: 002b
crash>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/8] mm/writeback: Throttle based on page writeback instead of congestion
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
` (2 preceding siblings ...)
2021-10-22 14:46 ` [PATCH 3/8] mm/vmscan: Throttle reclaim when no progress is being made Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-22 14:46 ` [PATCH 5/8] mm/page_alloc: Remove the throttling logic from the page allocator Mel Gorman
` (3 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
do_writepages throttles on congestion if the writepages() fails due to a
lack of memory but congestion_wait() is partially broken as the congestion
state is not updated for all BDIs.
This patch stalls waiting for a number of pages to complete writeback
that located on the local node. The main weakness is that there is no
correlation between the location of the inode's pages and locality but
that is still better than congestion_wait.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/page-writeback.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 4812a17b288c..f34f54fcd5b4 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2366,8 +2366,15 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
ret = generic_writepages(mapping, wbc);
if ((ret != -ENOMEM) || (wbc->sync_mode != WB_SYNC_ALL))
break;
- cond_resched();
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+
+ /*
+ * Lacking an allocation context or the locality or writeback
+ * state of any of the inode's pages, throttle based on
+ * writeback activity on the local node. It's as good a
+ * guess as any.
+ */
+ reclaim_throttle(NODE_DATA(numa_node_id()),
+ VMSCAN_THROTTLE_WRITEBACK, HZ/50);
}
/*
* Usually few pages are written by now from those we've just submitted
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/8] mm/page_alloc: Remove the throttling logic from the page allocator
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
` (3 preceding siblings ...)
2021-10-22 14:46 ` [PATCH 4/8] mm/writeback: Throttle based on page writeback instead of congestion Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-25 10:07 ` Vlastimil Babka
2021-10-22 14:46 ` [PATCH 6/8] mm/vmscan: Centralise timeout values for reclaim_throttle Mel Gorman
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
The page allocator stalls based on the number of pages that are
waiting for writeback to start but this should now be redundant.
shrink_inactive_list() will wake flusher threads if the LRU tail are
unqueued dirty pages so the flusher should be active. If it fails to make
progress due to pages under writeback not being completed quickly then
it should stall on VMSCAN_THROTTLE_WRITEBACK.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
mm/page_alloc.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 78e538067651..8fa0109ff417 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4795,30 +4795,11 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
trace_reclaim_retry_zone(z, order, reclaimable,
available, min_wmark, *no_progress_loops, wmark);
if (wmark) {
- /*
- * If we didn't make any progress and have a lot of
- * dirty + writeback pages then we should wait for
- * an IO to complete to slow down the reclaim and
- * prevent from pre mature OOM
- */
- if (!did_some_progress) {
- unsigned long write_pending;
-
- write_pending = zone_page_state_snapshot(zone,
- NR_ZONE_WRITE_PENDING);
-
- if (2 * write_pending > reclaimable) {
- congestion_wait(BLK_RW_ASYNC, HZ/10);
- return true;
- }
- }
-
ret = true;
- goto out;
+ break;
}
}
-out:
/*
* Memory allocation/reclaim might be called from a WQ context and the
* current implementation of the WQ concurrency control doesn't
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 5/8] mm/page_alloc: Remove the throttling logic from the page allocator
2021-10-22 14:46 ` [PATCH 5/8] mm/page_alloc: Remove the throttling logic from the page allocator Mel Gorman
@ 2021-10-25 10:07 ` Vlastimil Babka
0 siblings, 0 replies; 23+ messages in thread
From: Vlastimil Babka @ 2021-10-25 10:07 UTC (permalink / raw)
To: Mel Gorman, Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Johannes Weiner, Jonathan Corbet, Linux-MM, Linux-fsdevel, LKML
On 10/22/21 16:46, Mel Gorman wrote:
> The page allocator stalls based on the number of pages that are
> waiting for writeback to start but this should now be redundant.
> shrink_inactive_list() will wake flusher threads if the LRU tail are
> unqueued dirty pages so the flusher should be active. If it fails to make
> progress due to pages under writeback not being completed quickly then
> it should stall on VMSCAN_THROTTLE_WRITEBACK.
>
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/page_alloc.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 78e538067651..8fa0109ff417 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4795,30 +4795,11 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
> trace_reclaim_retry_zone(z, order, reclaimable,
> available, min_wmark, *no_progress_loops, wmark);
> if (wmark) {
> - /*
> - * If we didn't make any progress and have a lot of
> - * dirty + writeback pages then we should wait for
> - * an IO to complete to slow down the reclaim and
> - * prevent from pre mature OOM
> - */
> - if (!did_some_progress) {
> - unsigned long write_pending;
> -
> - write_pending = zone_page_state_snapshot(zone,
> - NR_ZONE_WRITE_PENDING);
> -
> - if (2 * write_pending > reclaimable) {
> - congestion_wait(BLK_RW_ASYNC, HZ/10);
> - return true;
> - }
> - }
> -
> ret = true;
> - goto out;
> + break;
> }
> }
>
> -out:
> /*
> * Memory allocation/reclaim might be called from a WQ context and the
> * current implementation of the WQ concurrency control doesn't
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 6/8] mm/vmscan: Centralise timeout values for reclaim_throttle
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
` (4 preceding siblings ...)
2021-10-22 14:46 ` [PATCH 5/8] mm/page_alloc: Remove the throttling logic from the page allocator Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-22 14:46 ` [PATCH 7/8] mm/vmscan: Increase the timeout if page reclaim is not making progress Mel Gorman
2021-10-22 14:46 ` [PATCH 8/8] mm/vmscan: Delay waking of tasks throttled on NOPROGRESS Mel Gorman
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Neil Brown raised concerns about callers of reclaim_throttle specifying
a timeout value. The original timeout values to congestion_wait() were
probably pulled out of thin air or copy&pasted from somewhere else.
This patch centralises the timeout values and selects a timeout based
on the reason for reclaim throttling. These figures are also pulled
out of the same thin air but better values may be derived
Running a workload that is throttling for inappropriate periods
and tracing mm_vmscan_throttled can be used to pick a more appropriate
value. Excessive throttling would pick a lower timeout where as
excessive CPU usage in reclaim context would select a larger timeout.
Ideally a large value would always be used and the wakeups would
occur before a timeout but that requires careful testing.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/compaction.c | 2 +-
mm/internal.h | 3 +--
mm/page-writeback.c | 2 +-
mm/vmscan.c | 50 +++++++++++++++++++++++++++++++++------------
4 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 7359093d8ac0..151b04c4dab3 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -828,7 +828,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
if (cc->mode == MIGRATE_ASYNC)
return -EAGAIN;
- reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
if (fatal_signal_pending(current))
return -EINTR;
diff --git a/mm/internal.h b/mm/internal.h
index c72d3383ef34..383d9b7e7991 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -129,8 +129,7 @@ extern unsigned long highest_memmap_pfn;
*/
extern int isolate_lru_page(struct page *page);
extern void putback_lru_page(struct page *page);
-extern void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
- long timeout);
+extern void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason);
/*
* in mm/rmap.c:
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index f34f54fcd5b4..4b01a6872f9e 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2374,7 +2374,7 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
* guess as any.
*/
reclaim_throttle(NODE_DATA(numa_node_id()),
- VMSCAN_THROTTLE_WRITEBACK, HZ/50);
+ VMSCAN_THROTTLE_WRITEBACK);
}
/*
* Usually few pages are written by now from those we've just submitted
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0450f6867d61..66da45084af4 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1006,12 +1006,10 @@ static void handle_write_error(struct address_space *mapping,
unlock_page(page);
}
-void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
- long timeout)
+void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
{
wait_queue_head_t *wqh = &pgdat->reclaim_wait[reason];
- long ret;
- bool acct_writeback = (reason == VMSCAN_THROTTLE_WRITEBACK);
+ long timeout, ret;
DEFINE_WAIT(wait);
/*
@@ -1023,17 +1021,43 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
current->flags & (PF_IO_WORKER|PF_KTHREAD))
return;
- if (acct_writeback &&
- atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
- WRITE_ONCE(pgdat->nr_reclaim_start,
- node_page_state(pgdat, NR_THROTTLED_WRITTEN));
+ /*
+ * These figures are pulled out of thin air.
+ * VMSCAN_THROTTLE_ISOLATED is a transient condition based on too many
+ * parallel reclaimers which is a short-lived event so the timeout is
+ * short. Failing to make progress or waiting on writeback are
+ * potentially long-lived events so use a longer timeout. This is shaky
+ * logic as a failure to make progress could be due to anything from
+ * writeback to a slow device to excessive references pages at the tail
+ * of the inactive LRU.
+ */
+ switch(reason) {
+ case VMSCAN_THROTTLE_WRITEBACK:
+ timeout = HZ/10;
+
+ if (atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
+ WRITE_ONCE(pgdat->nr_reclaim_start,
+ node_page_state(pgdat, NR_THROTTLED_WRITTEN));
+ }
+
+ break;
+ case VMSCAN_THROTTLE_NOPROGRESS:
+ timeout = HZ/10;
+ break;
+ case VMSCAN_THROTTLE_ISOLATED:
+ timeout = HZ/50;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ timeout = HZ;
+ break;
}
prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
ret = schedule_timeout(timeout);
finish_wait(wqh, &wait);
- if (acct_writeback)
+ if (reason == VMSCAN_THROTTLE_WRITEBACK)
atomic_dec(&pgdat->nr_writeback_throttled);
trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
@@ -2319,7 +2343,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
/* wait a bit for the reclaimer. */
stalled = true;
- reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
/* We are about to die and free our memory. Return now. */
if (fatal_signal_pending(current))
@@ -3251,7 +3275,7 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
* until some pages complete writeback.
*/
if (sc->nr.immediate)
- reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK);
}
/*
@@ -3275,7 +3299,7 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
if (!current_is_kswapd() && current_may_throttle() &&
!sc->hibernation_mode &&
test_bit(LRUVEC_CONGESTED, &target_lruvec->flags))
- reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK);
if (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
sc))
@@ -3347,7 +3371,7 @@ static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
/* Throttle if making no progress at high prioities. */
if (sc->priority < DEF_PRIORITY - 2)
- reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_NOPROGRESS);
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 7/8] mm/vmscan: Increase the timeout if page reclaim is not making progress
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
` (5 preceding siblings ...)
2021-10-22 14:46 ` [PATCH 6/8] mm/vmscan: Centralise timeout values for reclaim_throttle Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
2021-10-22 14:46 ` [PATCH 8/8] mm/vmscan: Delay waking of tasks throttled on NOPROGRESS Mel Gorman
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Tracing of the stutterp workload showed the following delays
1 usect_delayed=124000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=128000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=176000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=536000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=544000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=556000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=624000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=716000 reason=VMSCAN_THROTTLE_NOPROGRESS
1 usect_delayed=772000 reason=VMSCAN_THROTTLE_NOPROGRESS
2 usect_delayed=512000 reason=VMSCAN_THROTTLE_NOPROGRESS
16 usect_delayed=120000 reason=VMSCAN_THROTTLE_NOPROGRESS
53 usect_delayed=116000 reason=VMSCAN_THROTTLE_NOPROGRESS
116 usect_delayed=112000 reason=VMSCAN_THROTTLE_NOPROGRESS
5907 usect_delayed=108000 reason=VMSCAN_THROTTLE_NOPROGRESS
71741 usect_delayed=104000 reason=VMSCAN_THROTTLE_NOPROGRESS
All the throttling hit the full timeout and then there was wakeup delays
meaning that the wakeups are premature as no other reclaimer such as
kswapd has made progress. This patch increases the maximum timeout.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/vmscan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 66da45084af4..35b6ccaa01c3 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1042,7 +1042,7 @@ void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason)
break;
case VMSCAN_THROTTLE_NOPROGRESS:
- timeout = HZ/10;
+ timeout = HZ/2;
break;
case VMSCAN_THROTTLE_ISOLATED:
timeout = HZ/50;
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 8/8] mm/vmscan: Delay waking of tasks throttled on NOPROGRESS
2021-10-22 14:46 [PATCH v5 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
` (6 preceding siblings ...)
2021-10-22 14:46 ` [PATCH 7/8] mm/vmscan: Increase the timeout if page reclaim is not making progress Mel Gorman
@ 2021-10-22 14:46 ` Mel Gorman
7 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-22 14:46 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Tracing indicates that tasks throttled on NOPROGRESS are woken
prematurely resulting in occasional massive spikes in direct
reclaim activity. This patch wakes tasks throttled on NOPROGRESS
if reclaim efficiency is at least 12%.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/vmscan.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 35b6ccaa01c3..812d4697d50d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3349,8 +3349,11 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
static void consider_reclaim_throttle(pg_data_t *pgdat, struct scan_control *sc)
{
- /* If reclaim is making progress, wake any throttled tasks. */
- if (sc->nr_reclaimed) {
+ /*
+ * If reclaim is making progress greater than 12% efficiency then
+ * wake all the NOPROGRESS throttled tasks.
+ */
+ if (sc->nr_reclaimed > (sc->nr_scanned >> 3)) {
wait_queue_head_t *wqh;
wqh = &pgdat->reclaim_wait[VMSCAN_THROTTLE_NOPROGRESS];
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-19 9:01 [PATCH v4 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
@ 2021-10-19 9:01 ` Mel Gorman
0 siblings, 0 replies; 23+ messages in thread
From: Mel Gorman @ 2021-10-19 9:01 UTC (permalink / raw)
To: Andrew Morton
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-MM,
Linux-fsdevel, LKML, Mel Gorman
Page reclaim throttles on wait_iff_congested under the following conditions
o kswapd is encountering pages under writeback and marked for immediate
reclaim implying that pages are cycling through the LRU faster than
pages can be cleaned.
o Direct reclaim will stall if all dirty pages are backed by congested
inodes.
wait_iff_congested is almost completely broken with few exceptions. This
patch adds a new node-based workqueue and tracks the number of throttled
tasks and pages written back since throttling started. If enough pages
belonging to the node are written back then the throttled tasks will wake
early. If not, the throttled tasks sleeps until the timeout expires.
[neilb@suse.de: Uninterruptible sleep and simpler wakeups]
[hdanton@sina.com: Avoid race when reclaim starts]
[vbabka@suse.cz: vmstat irq-safe api, clarifications]
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/backing-dev.h | 1 -
include/linux/mmzone.h | 9 ++++
include/trace/events/vmscan.h | 34 +++++++++++++
include/trace/events/writeback.h | 7 ---
mm/backing-dev.c | 48 -------------------
mm/filemap.c | 1 +
mm/internal.h | 11 +++++
mm/page_alloc.c | 1 +
mm/vmscan.c | 82 +++++++++++++++++++++++++++-----
mm/vmstat.c | 1 +
10 files changed, 127 insertions(+), 68 deletions(-)
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index ac7f231b8825..9fb1f0ae273c 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -154,7 +154,6 @@ static inline int wb_congested(struct bdi_writeback *wb, int cong_bits)
}
long congestion_wait(int sync, long timeout);
-long wait_iff_congested(int sync, long timeout);
static inline bool mapping_can_writeback(struct address_space *mapping)
{
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6a1d79d84675..ef0a63ebd21d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -199,6 +199,7 @@ enum node_stat_item {
NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */
NR_DIRTIED, /* page dirtyings since bootup */
NR_WRITTEN, /* page writings since bootup */
+ NR_THROTTLED_WRITTEN, /* NR_WRITTEN while reclaim throttled */
NR_KERNEL_MISC_RECLAIMABLE, /* reclaimable non-slab kernel pages */
NR_FOLL_PIN_ACQUIRED, /* via: pin_user_page(), gup flag: FOLL_PIN */
NR_FOLL_PIN_RELEASED, /* pages returned via unpin_user_page() */
@@ -272,6 +273,10 @@ enum lru_list {
NR_LRU_LISTS
};
+enum vmscan_throttle_state {
+ VMSCAN_THROTTLE_WRITEBACK,
+};
+
#define for_each_lru(lru) for (lru = 0; lru < NR_LRU_LISTS; lru++)
#define for_each_evictable_lru(lru) for (lru = 0; lru <= LRU_ACTIVE_FILE; lru++)
@@ -841,6 +846,10 @@ typedef struct pglist_data {
int node_id;
wait_queue_head_t kswapd_wait;
wait_queue_head_t pfmemalloc_wait;
+ wait_queue_head_t reclaim_wait; /* wq for throttling reclaim */
+ atomic_t nr_reclaim_throttled; /* nr of throtted tasks */
+ unsigned long nr_reclaim_start; /* nr pages written while throttled
+ * when throttling started. */
struct task_struct *kswapd; /* Protected by
mem_hotplug_begin/end() */
int kswapd_order;
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 88faf2400ec2..c317f9fe0d17 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -27,6 +27,14 @@
{RECLAIM_WB_ASYNC, "RECLAIM_WB_ASYNC"} \
) : "RECLAIM_WB_NONE"
+#define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
+
+#define show_throttle_flags(flags) \
+ (flags) ? __print_flags(flags, "|", \
+ {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"} \
+ ) : "VMSCAN_THROTTLE_NONE"
+
+
#define trace_reclaim_flags(file) ( \
(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
(RECLAIM_WB_ASYNC) \
@@ -454,6 +462,32 @@ DEFINE_EVENT(mm_vmscan_direct_reclaim_end_template, mm_vmscan_node_reclaim_end,
TP_ARGS(nr_reclaimed)
);
+TRACE_EVENT(mm_vmscan_throttled,
+
+ TP_PROTO(int nid, int usec_timeout, int usec_delayed, int reason),
+
+ TP_ARGS(nid, usec_timeout, usec_delayed, reason),
+
+ TP_STRUCT__entry(
+ __field(int, nid)
+ __field(int, usec_timeout)
+ __field(int, usec_delayed)
+ __field(int, reason)
+ ),
+
+ TP_fast_assign(
+ __entry->nid = nid;
+ __entry->usec_timeout = usec_timeout;
+ __entry->usec_delayed = usec_delayed;
+ __entry->reason = 1U << reason;
+ ),
+
+ TP_printk("nid=%d usec_timeout=%d usect_delayed=%d reason=%s",
+ __entry->nid,
+ __entry->usec_timeout,
+ __entry->usec_delayed,
+ show_throttle_flags(__entry->reason))
+);
#endif /* _TRACE_VMSCAN_H */
/* This part must be outside protection */
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 840d1ba84cf5..3bc759b81897 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -763,13 +763,6 @@ DEFINE_EVENT(writeback_congest_waited_template, writeback_congestion_wait,
TP_ARGS(usec_timeout, usec_delayed)
);
-DEFINE_EVENT(writeback_congest_waited_template, writeback_wait_iff_congested,
-
- TP_PROTO(unsigned int usec_timeout, unsigned int usec_delayed),
-
- TP_ARGS(usec_timeout, usec_delayed)
-);
-
DECLARE_EVENT_CLASS(writeback_single_inode_template,
TP_PROTO(struct inode *inode,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 4a9d4e27d0d9..0ea1a105eae5 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -1041,51 +1041,3 @@ long congestion_wait(int sync, long timeout)
return ret;
}
EXPORT_SYMBOL(congestion_wait);
-
-/**
- * wait_iff_congested - Conditionally wait for a backing_dev to become uncongested or a pgdat to complete writes
- * @sync: SYNC or ASYNC IO
- * @timeout: timeout in jiffies
- *
- * In the event of a congested backing_dev (any backing_dev) this waits
- * for up to @timeout jiffies for either a BDI to exit congestion of the
- * given @sync queue or a write to complete.
- *
- * The return value is 0 if the sleep is for the full timeout. Otherwise,
- * it is the number of jiffies that were still remaining when the function
- * returned. return_value == timeout implies the function did not sleep.
- */
-long wait_iff_congested(int sync, long timeout)
-{
- long ret;
- unsigned long start = jiffies;
- DEFINE_WAIT(wait);
- wait_queue_head_t *wqh = &congestion_wqh[sync];
-
- /*
- * If there is no congestion, yield if necessary instead
- * of sleeping on the congestion queue
- */
- if (atomic_read(&nr_wb_congested[sync]) == 0) {
- cond_resched();
-
- /* In case we scheduled, work out time remaining */
- ret = timeout - (jiffies - start);
- if (ret < 0)
- ret = 0;
-
- goto out;
- }
-
- /* Sleep until uncongested or a write happens */
- prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
- ret = io_schedule_timeout(timeout);
- finish_wait(wqh, &wait);
-
-out:
- trace_writeback_wait_iff_congested(jiffies_to_usecs(timeout),
- jiffies_to_usecs(jiffies - start));
-
- return ret;
-}
-EXPORT_SYMBOL(wait_iff_congested);
diff --git a/mm/filemap.c b/mm/filemap.c
index dae481293b5d..59187787fbfc 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1605,6 +1605,7 @@ void end_page_writeback(struct page *page)
smp_mb__after_atomic();
wake_up_page(page, PG_writeback);
+ acct_reclaim_writeback(page);
put_page(page);
}
EXPORT_SYMBOL(end_page_writeback);
diff --git a/mm/internal.h b/mm/internal.h
index cf3cb933eba3..90764d646e02 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -34,6 +34,17 @@
void page_writeback_init(void);
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled);
+static inline void acct_reclaim_writeback(struct page *page)
+{
+ pg_data_t *pgdat = page_pgdat(page);
+ int nr_throttled = atomic_read(&pgdat->nr_reclaim_throttled);
+
+ if (nr_throttled)
+ __acct_reclaim_writeback(pgdat, page, nr_throttled);
+}
+
vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b37435c274cf..d849ddfc1e51 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7396,6 +7396,7 @@ static void __meminit pgdat_init_internals(struct pglist_data *pgdat)
init_waitqueue_head(&pgdat->kswapd_wait);
init_waitqueue_head(&pgdat->pfmemalloc_wait);
+ init_waitqueue_head(&pgdat->reclaim_wait);
pgdat_page_ext_init(pgdat);
lruvec_init(&pgdat->__lruvec);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 74296c2d1fed..735b1f2b5d9e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1006,6 +1006,64 @@ static void handle_write_error(struct address_space *mapping,
unlock_page(page);
}
+static void
+reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
+ long timeout)
+{
+ wait_queue_head_t *wqh = &pgdat->reclaim_wait;
+ long ret;
+ DEFINE_WAIT(wait);
+
+ /*
+ * Do not throttle IO workers, kthreads other than kswapd or
+ * workqueues. They may be required for reclaim to make
+ * forward progress (e.g. journalling workqueues or kthreads).
+ */
+ if (!current_is_kswapd() &&
+ current->flags & (PF_IO_WORKER|PF_KTHREAD))
+ return;
+
+ if (atomic_inc_return(&pgdat->nr_reclaim_throttled) == 1) {
+ WRITE_ONCE(pgdat->nr_reclaim_start,
+ node_page_state(pgdat, NR_THROTTLED_WRITTEN));
+ }
+
+ prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+ ret = schedule_timeout(timeout);
+ finish_wait(wqh, &wait);
+ atomic_dec(&pgdat->nr_reclaim_throttled);
+
+ trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
+ jiffies_to_usecs(timeout - ret),
+ reason);
+}
+
+/*
+ * Account for pages written if tasks are throttled waiting on dirty
+ * pages to clean. If enough pages have been cleaned since throttling
+ * started then wakeup the throttled tasks.
+ */
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled)
+{
+ unsigned long nr_written;
+
+ inc_node_page_state(page, NR_THROTTLED_WRITTEN);
+
+ /*
+ * This is an inaccurate read as the per-cpu deltas may not
+ * be synchronised. However, given that the system is
+ * writeback throttled, it is not worth taking the penalty
+ * of getting an accurate count. At worst, the throttle
+ * timeout guarantees forward progress.
+ */
+ nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
+ READ_ONCE(pgdat->nr_reclaim_start);
+
+ if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
+ wake_up_all(&pgdat->reclaim_wait);
+}
+
/* possible outcome of pageout() */
typedef enum {
/* failed to write page out, page is locked */
@@ -1412,9 +1470,8 @@ static unsigned int shrink_page_list(struct list_head *page_list,
/*
* The number of dirty pages determines if a node is marked
- * reclaim_congested which affects wait_iff_congested. kswapd
- * will stall and start writing pages if the tail of the LRU
- * is all dirty unqueued pages.
+ * reclaim_congested. kswapd will stall and start writing
+ * pages if the tail of the LRU is all dirty unqueued pages.
*/
page_check_dirty_writeback(page, &dirty, &writeback);
if (dirty || writeback)
@@ -3180,19 +3237,19 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
* If kswapd scans pages marked for immediate
* reclaim and under writeback (nr_immediate), it
* implies that pages are cycling through the LRU
- * faster than they are written so also forcibly stall.
+ * faster than they are written so forcibly stall
+ * until some pages complete writeback.
*/
if (sc->nr.immediate)
- congestion_wait(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
}
/*
- * Tag a node/memcg as congested if all the dirty pages
- * scanned were backed by a congested BDI and
- * wait_iff_congested will stall.
+ * Tag a node/memcg as congested if all the dirty pages were marked
+ * for writeback and immediate reclaim (counted in nr.congested).
*
* Legacy memcg will stall in page writeback so avoid forcibly
- * stalling in wait_iff_congested().
+ * stalling in reclaim_throttle().
*/
if ((current_is_kswapd() ||
(cgroup_reclaim(sc) && writeback_throttling_sane(sc))) &&
@@ -3200,15 +3257,15 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
set_bit(LRUVEC_CONGESTED, &target_lruvec->flags);
/*
- * Stall direct reclaim for IO completions if underlying BDIs
- * and node is congested. Allow kswapd to continue until it
+ * Stall direct reclaim for IO completions if the lruvec is
+ * node is congested. Allow kswapd to continue until it
* starts encountering unqueued dirty pages or cycling through
* the LRU too quickly.
*/
if (!current_is_kswapd() && current_may_throttle() &&
!sc->hibernation_mode &&
test_bit(LRUVEC_CONGESTED, &target_lruvec->flags))
- wait_iff_congested(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
if (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
sc))
@@ -4286,6 +4343,7 @@ static int kswapd(void *p)
WRITE_ONCE(pgdat->kswapd_order, 0);
WRITE_ONCE(pgdat->kswapd_highest_zoneidx, MAX_NR_ZONES);
+ atomic_set(&pgdat->nr_reclaim_throttled, 0);
for ( ; ; ) {
bool ret;
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 8ce2620344b2..9b2bc9d61d4b 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1225,6 +1225,7 @@ const char * const vmstat_text[] = {
"nr_vmscan_immediate_reclaim",
"nr_dirtied",
"nr_written",
+ "nr_throttled_written",
"nr_kernel_misc_reclaimable",
"nr_foll_pin_acquired",
"nr_foll_pin_released",
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-08 13:53 [PATCH v3 0/8] Remove dependency on congestion_wait in mm/ Mel Gorman
@ 2021-10-08 13:53 ` Mel Gorman
2021-10-13 15:39 ` Vlastimil Babka
0 siblings, 1 reply; 23+ messages in thread
From: Mel Gorman @ 2021-10-08 13:53 UTC (permalink / raw)
To: Linux-MM
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Vlastimil Babka, Johannes Weiner, Jonathan Corbet, Linux-fsdevel,
LKML, Mel Gorman
Page reclaim throttles on wait_iff_congested under the following conditions
o kswapd is encountering pages under writeback and marked for immediate
reclaim implying that pages are cycling through the LRU faster than
pages can be cleaned.
o Direct reclaim will stall if all dirty pages are backed by congested
inodes.
wait_iff_congested is almost completely broken with few exceptions. This
patch adds a new node-based workqueue and tracks the number of throttled
tasks and pages written back since throttling started. If enough pages
belonging to the node are written back then the throttled tasks will wake
early. If not, the throttled tasks sleeps until the timeout expires.
[neilb@suse.de: Uninterruptible sleep and simpler wakeups]
[hdanton@sina.com: Avoid race when reclaim starts]
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
include/linux/backing-dev.h | 1 -
include/linux/mmzone.h | 9 ++++
include/trace/events/vmscan.h | 34 +++++++++++++++
include/trace/events/writeback.h | 7 ---
mm/backing-dev.c | 48 ---------------------
mm/filemap.c | 1 +
mm/internal.h | 11 +++++
mm/page_alloc.c | 1 +
mm/vmscan.c | 74 ++++++++++++++++++++++++++------
mm/vmstat.c | 1 +
10 files changed, 119 insertions(+), 68 deletions(-)
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index ac7f231b8825..9fb1f0ae273c 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -154,7 +154,6 @@ static inline int wb_congested(struct bdi_writeback *wb, int cong_bits)
}
long congestion_wait(int sync, long timeout);
-long wait_iff_congested(int sync, long timeout);
static inline bool mapping_can_writeback(struct address_space *mapping)
{
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6a1d79d84675..ef0a63ebd21d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -199,6 +199,7 @@ enum node_stat_item {
NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */
NR_DIRTIED, /* page dirtyings since bootup */
NR_WRITTEN, /* page writings since bootup */
+ NR_THROTTLED_WRITTEN, /* NR_WRITTEN while reclaim throttled */
NR_KERNEL_MISC_RECLAIMABLE, /* reclaimable non-slab kernel pages */
NR_FOLL_PIN_ACQUIRED, /* via: pin_user_page(), gup flag: FOLL_PIN */
NR_FOLL_PIN_RELEASED, /* pages returned via unpin_user_page() */
@@ -272,6 +273,10 @@ enum lru_list {
NR_LRU_LISTS
};
+enum vmscan_throttle_state {
+ VMSCAN_THROTTLE_WRITEBACK,
+};
+
#define for_each_lru(lru) for (lru = 0; lru < NR_LRU_LISTS; lru++)
#define for_each_evictable_lru(lru) for (lru = 0; lru <= LRU_ACTIVE_FILE; lru++)
@@ -841,6 +846,10 @@ typedef struct pglist_data {
int node_id;
wait_queue_head_t kswapd_wait;
wait_queue_head_t pfmemalloc_wait;
+ wait_queue_head_t reclaim_wait; /* wq for throttling reclaim */
+ atomic_t nr_reclaim_throttled; /* nr of throtted tasks */
+ unsigned long nr_reclaim_start; /* nr pages written while throttled
+ * when throttling started. */
struct task_struct *kswapd; /* Protected by
mem_hotplug_begin/end() */
int kswapd_order;
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 88faf2400ec2..c317f9fe0d17 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -27,6 +27,14 @@
{RECLAIM_WB_ASYNC, "RECLAIM_WB_ASYNC"} \
) : "RECLAIM_WB_NONE"
+#define _VMSCAN_THROTTLE_WRITEBACK (1 << VMSCAN_THROTTLE_WRITEBACK)
+
+#define show_throttle_flags(flags) \
+ (flags) ? __print_flags(flags, "|", \
+ {_VMSCAN_THROTTLE_WRITEBACK, "VMSCAN_THROTTLE_WRITEBACK"} \
+ ) : "VMSCAN_THROTTLE_NONE"
+
+
#define trace_reclaim_flags(file) ( \
(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
(RECLAIM_WB_ASYNC) \
@@ -454,6 +462,32 @@ DEFINE_EVENT(mm_vmscan_direct_reclaim_end_template, mm_vmscan_node_reclaim_end,
TP_ARGS(nr_reclaimed)
);
+TRACE_EVENT(mm_vmscan_throttled,
+
+ TP_PROTO(int nid, int usec_timeout, int usec_delayed, int reason),
+
+ TP_ARGS(nid, usec_timeout, usec_delayed, reason),
+
+ TP_STRUCT__entry(
+ __field(int, nid)
+ __field(int, usec_timeout)
+ __field(int, usec_delayed)
+ __field(int, reason)
+ ),
+
+ TP_fast_assign(
+ __entry->nid = nid;
+ __entry->usec_timeout = usec_timeout;
+ __entry->usec_delayed = usec_delayed;
+ __entry->reason = 1U << reason;
+ ),
+
+ TP_printk("nid=%d usec_timeout=%d usect_delayed=%d reason=%s",
+ __entry->nid,
+ __entry->usec_timeout,
+ __entry->usec_delayed,
+ show_throttle_flags(__entry->reason))
+);
#endif /* _TRACE_VMSCAN_H */
/* This part must be outside protection */
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 840d1ba84cf5..3bc759b81897 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -763,13 +763,6 @@ DEFINE_EVENT(writeback_congest_waited_template, writeback_congestion_wait,
TP_ARGS(usec_timeout, usec_delayed)
);
-DEFINE_EVENT(writeback_congest_waited_template, writeback_wait_iff_congested,
-
- TP_PROTO(unsigned int usec_timeout, unsigned int usec_delayed),
-
- TP_ARGS(usec_timeout, usec_delayed)
-);
-
DECLARE_EVENT_CLASS(writeback_single_inode_template,
TP_PROTO(struct inode *inode,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 4a9d4e27d0d9..0ea1a105eae5 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -1041,51 +1041,3 @@ long congestion_wait(int sync, long timeout)
return ret;
}
EXPORT_SYMBOL(congestion_wait);
-
-/**
- * wait_iff_congested - Conditionally wait for a backing_dev to become uncongested or a pgdat to complete writes
- * @sync: SYNC or ASYNC IO
- * @timeout: timeout in jiffies
- *
- * In the event of a congested backing_dev (any backing_dev) this waits
- * for up to @timeout jiffies for either a BDI to exit congestion of the
- * given @sync queue or a write to complete.
- *
- * The return value is 0 if the sleep is for the full timeout. Otherwise,
- * it is the number of jiffies that were still remaining when the function
- * returned. return_value == timeout implies the function did not sleep.
- */
-long wait_iff_congested(int sync, long timeout)
-{
- long ret;
- unsigned long start = jiffies;
- DEFINE_WAIT(wait);
- wait_queue_head_t *wqh = &congestion_wqh[sync];
-
- /*
- * If there is no congestion, yield if necessary instead
- * of sleeping on the congestion queue
- */
- if (atomic_read(&nr_wb_congested[sync]) == 0) {
- cond_resched();
-
- /* In case we scheduled, work out time remaining */
- ret = timeout - (jiffies - start);
- if (ret < 0)
- ret = 0;
-
- goto out;
- }
-
- /* Sleep until uncongested or a write happens */
- prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
- ret = io_schedule_timeout(timeout);
- finish_wait(wqh, &wait);
-
-out:
- trace_writeback_wait_iff_congested(jiffies_to_usecs(timeout),
- jiffies_to_usecs(jiffies - start));
-
- return ret;
-}
-EXPORT_SYMBOL(wait_iff_congested);
diff --git a/mm/filemap.c b/mm/filemap.c
index dae481293b5d..59187787fbfc 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1605,6 +1605,7 @@ void end_page_writeback(struct page *page)
smp_mb__after_atomic();
wake_up_page(page, PG_writeback);
+ acct_reclaim_writeback(page);
put_page(page);
}
EXPORT_SYMBOL(end_page_writeback);
diff --git a/mm/internal.h b/mm/internal.h
index cf3cb933eba3..90764d646e02 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -34,6 +34,17 @@
void page_writeback_init(void);
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled);
+static inline void acct_reclaim_writeback(struct page *page)
+{
+ pg_data_t *pgdat = page_pgdat(page);
+ int nr_throttled = atomic_read(&pgdat->nr_reclaim_throttled);
+
+ if (nr_throttled)
+ __acct_reclaim_writeback(pgdat, page, nr_throttled);
+}
+
vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b37435c274cf..d849ddfc1e51 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7396,6 +7396,7 @@ static void __meminit pgdat_init_internals(struct pglist_data *pgdat)
init_waitqueue_head(&pgdat->kswapd_wait);
init_waitqueue_head(&pgdat->pfmemalloc_wait);
+ init_waitqueue_head(&pgdat->reclaim_wait);
pgdat_page_ext_init(pgdat);
lruvec_init(&pgdat->__lruvec);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 74296c2d1fed..bcd22e53795f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1006,6 +1006,56 @@ static void handle_write_error(struct address_space *mapping,
unlock_page(page);
}
+static void
+reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
+ long timeout)
+{
+ wait_queue_head_t *wqh = &pgdat->reclaim_wait;
+ long ret;
+ DEFINE_WAIT(wait);
+
+ /*
+ * Do not throttle IO workers, kthreads other than kswapd or
+ * workqueues. They may be required for reclaim to make
+ * forward progress (e.g. journalling workqueues or kthreads).
+ */
+ if (!current_is_kswapd() &&
+ current->flags & (PF_IO_WORKER|PF_KTHREAD))
+ return;
+
+ if (atomic_inc_return(&pgdat->nr_reclaim_throttled) == 1) {
+ WRITE_ONCE(pgdat->nr_reclaim_start,
+ node_page_state(pgdat, NR_THROTTLED_WRITTEN));
+ }
+
+ prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+ ret = schedule_timeout(timeout);
+ finish_wait(wqh, &wait);
+ atomic_dec(&pgdat->nr_reclaim_throttled);
+
+ trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
+ jiffies_to_usecs(timeout - ret),
+ reason);
+}
+
+/*
+ * Account for pages written if tasks are throttled waiting on dirty
+ * pages to clean. If enough pages have been cleaned since throttling
+ * started then wakeup the throttled tasks.
+ */
+void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
+ int nr_throttled)
+{
+ unsigned long nr_written;
+
+ __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
+ nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
+ READ_ONCE(pgdat->nr_reclaim_start);
+
+ if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
+ wake_up_all(&pgdat->reclaim_wait);
+}
+
/* possible outcome of pageout() */
typedef enum {
/* failed to write page out, page is locked */
@@ -1412,9 +1462,8 @@ static unsigned int shrink_page_list(struct list_head *page_list,
/*
* The number of dirty pages determines if a node is marked
- * reclaim_congested which affects wait_iff_congested. kswapd
- * will stall and start writing pages if the tail of the LRU
- * is all dirty unqueued pages.
+ * reclaim_congested. kswapd will stall and start writing
+ * pages if the tail of the LRU is all dirty unqueued pages.
*/
page_check_dirty_writeback(page, &dirty, &writeback);
if (dirty || writeback)
@@ -3180,19 +3229,19 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
* If kswapd scans pages marked for immediate
* reclaim and under writeback (nr_immediate), it
* implies that pages are cycling through the LRU
- * faster than they are written so also forcibly stall.
+ * faster than they are written so forcibly stall
+ * until some pages complete writeback.
*/
if (sc->nr.immediate)
- congestion_wait(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
}
/*
- * Tag a node/memcg as congested if all the dirty pages
- * scanned were backed by a congested BDI and
- * wait_iff_congested will stall.
+ * Tag a node/memcg as congested if all the dirty pages were marked
+ * for writeback and immediate reclaim (counted in nr.congested).
*
* Legacy memcg will stall in page writeback so avoid forcibly
- * stalling in wait_iff_congested().
+ * stalling in reclaim_throttle().
*/
if ((current_is_kswapd() ||
(cgroup_reclaim(sc) && writeback_throttling_sane(sc))) &&
@@ -3200,15 +3249,15 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
set_bit(LRUVEC_CONGESTED, &target_lruvec->flags);
/*
- * Stall direct reclaim for IO completions if underlying BDIs
- * and node is congested. Allow kswapd to continue until it
+ * Stall direct reclaim for IO completions if the lruvec is
+ * node is congested. Allow kswapd to continue until it
* starts encountering unqueued dirty pages or cycling through
* the LRU too quickly.
*/
if (!current_is_kswapd() && current_may_throttle() &&
!sc->hibernation_mode &&
test_bit(LRUVEC_CONGESTED, &target_lruvec->flags))
- wait_iff_congested(BLK_RW_ASYNC, HZ/10);
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK, HZ/10);
if (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
sc))
@@ -4286,6 +4335,7 @@ static int kswapd(void *p)
WRITE_ONCE(pgdat->kswapd_order, 0);
WRITE_ONCE(pgdat->kswapd_highest_zoneidx, MAX_NR_ZONES);
+ atomic_set(&pgdat->nr_reclaim_throttled, 0);
for ( ; ; ) {
bool ret;
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 8ce2620344b2..9b2bc9d61d4b 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1225,6 +1225,7 @@ const char * const vmstat_text[] = {
"nr_vmscan_immediate_reclaim",
"nr_dirtied",
"nr_written",
+ "nr_throttled_written",
"nr_kernel_misc_reclaimable",
"nr_foll_pin_acquired",
"nr_foll_pin_released",
--
2.31.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-08 13:53 ` [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested Mel Gorman
@ 2021-10-13 15:39 ` Vlastimil Babka
2021-10-14 10:47 ` Mel Gorman
0 siblings, 1 reply; 23+ messages in thread
From: Vlastimil Babka @ 2021-10-13 15:39 UTC (permalink / raw)
To: Mel Gorman, Linux-MM
Cc: NeilBrown, Theodore Ts'o, Andreas Dilger, Darrick J . Wong,
Matthew Wilcox, Michal Hocko, Dave Chinner, Rik van Riel,
Johannes Weiner, Jonathan Corbet, Linux-fsdevel, LKML
On 10/8/21 15:53, Mel Gorman wrote:
> Page reclaim throttles on wait_iff_congested under the following conditions
>
> o kswapd is encountering pages under writeback and marked for immediate
> reclaim implying that pages are cycling through the LRU faster than
> pages can be cleaned.
>
> o Direct reclaim will stall if all dirty pages are backed by congested
> inodes.
>
> wait_iff_congested is almost completely broken with few exceptions. This
> patch adds a new node-based workqueue and tracks the number of throttled
> tasks and pages written back since throttling started. If enough pages
> belonging to the node are written back then the throttled tasks will wake
> early. If not, the throttled tasks sleeps until the timeout expires.
>
> [neilb@suse.de: Uninterruptible sleep and simpler wakeups]
> [hdanton@sina.com: Avoid race when reclaim starts]
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Seems mostly OK, have just some doubts wrt NR_THROTTLED_WRITTEN mechanics,
that may ultimately be just a point of comments to add.
...
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1006,6 +1006,56 @@ static void handle_write_error(struct address_space *mapping,
> unlock_page(page);
> }
>
> +static void
> +reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason,
> + long timeout)
> +{
> + wait_queue_head_t *wqh = &pgdat->reclaim_wait;
> + long ret;
> + DEFINE_WAIT(wait);
> +
> + /*
> + * Do not throttle IO workers, kthreads other than kswapd or
> + * workqueues. They may be required for reclaim to make
> + * forward progress (e.g. journalling workqueues or kthreads).
> + */
> + if (!current_is_kswapd() &&
> + current->flags & (PF_IO_WORKER|PF_KTHREAD))
> + return;
> +
> + if (atomic_inc_return(&pgdat->nr_reclaim_throttled) == 1) {
> + WRITE_ONCE(pgdat->nr_reclaim_start,
> + node_page_state(pgdat, NR_THROTTLED_WRITTEN));
> + }
> +
> + prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
> + ret = schedule_timeout(timeout);
> + finish_wait(wqh, &wait);
> + atomic_dec(&pgdat->nr_reclaim_throttled);
> +
> + trace_mm_vmscan_throttled(pgdat->node_id, jiffies_to_usecs(timeout),
> + jiffies_to_usecs(timeout - ret),
> + reason);
> +}
> +
> +/*
> + * Account for pages written if tasks are throttled waiting on dirty
> + * pages to clean. If enough pages have been cleaned since throttling
> + * started then wakeup the throttled tasks.
> + */
> +void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
> + int nr_throttled)
> +{
> + unsigned long nr_written;
> +
> + __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
Is this intentionally using the __ version that normally expects irqs to be
disabled (AFAIK they are not in this path)? I think this is rarely used cold
path so it doesn't seem worth to trade off speed for accuracy.
> + nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
> + READ_ONCE(pgdat->nr_reclaim_start);
Even if the inc above was safe, node_page_state() will return only the
global counter, so the value we read here will only actually increment when
some cpu's counter overflows, so it will be "bursty". Maybe it's ok, just
worth documenting?
> +
> + if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
> + wake_up_all(&pgdat->reclaim_wait);
Hm it seems a bit weird that the more tasks are throttled, the more we wait,
and then wake up all. Theoretically this will lead to even more
bursty/staggering herd behavior. Could be better to wake up single task each
SWAP_CLUSTER_MAX, and bump nr_reclaim_start? But maybe it's not a problem in
practice due to HZ/10 timeouts being short enough?
> +}
> +
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-13 15:39 ` Vlastimil Babka
@ 2021-10-14 10:47 ` Mel Gorman
2021-10-14 15:42 ` Vlastimil Babka
0 siblings, 1 reply; 23+ messages in thread
From: Mel Gorman @ 2021-10-14 10:47 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Linux-MM, NeilBrown, Theodore Ts'o, Andreas Dilger,
Darrick J . Wong, Matthew Wilcox, Michal Hocko, Dave Chinner,
Rik van Riel, Johannes Weiner, Jonathan Corbet, Linux-fsdevel,
LKML
Thanks Vlastimil
On Wed, Oct 13, 2021 at 05:39:36PM +0200, Vlastimil Babka wrote:
> > +/*
> > + * Account for pages written if tasks are throttled waiting on dirty
> > + * pages to clean. If enough pages have been cleaned since throttling
> > + * started then wakeup the throttled tasks.
> > + */
> > +void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
> > + int nr_throttled)
> > +{
> > + unsigned long nr_written;
> > +
> > + __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
>
> Is this intentionally using the __ version that normally expects irqs to be
> disabled (AFAIK they are not in this path)? I think this is rarely used cold
> path so it doesn't seem worth to trade off speed for accuracy.
>
It was intentional because IRQs can be disabled and if it's race-prone,
it's not overly problematic but you're right, better to be safe. I changed
it to the safe type as it's mostly free on x86, arm64 and s390 and for
other architectures, this is a slow path.
> > + nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
> > + READ_ONCE(pgdat->nr_reclaim_start);
>
> Even if the inc above was safe, node_page_state() will return only the
> global counter, so the value we read here will only actually increment when
> some cpu's counter overflows, so it will be "bursty". Maybe it's ok, just
> worth documenting?
>
I didn't think the penalty of doing an accurate read while writeback
throttled is worth it. I'll add a comment.
> > +
> > + if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
> > + wake_up_all(&pgdat->reclaim_wait);
>
> Hm it seems a bit weird that the more tasks are throttled, the more we wait,
> and then wake up all. Theoretically this will lead to even more
> bursty/staggering herd behavior. Could be better to wake up single task each
> SWAP_CLUSTER_MAX, and bump nr_reclaim_start? But maybe it's not a problem in
> practice due to HZ/10 timeouts being short enough?
>
Yes, the more tasks are throttled the longer tasks wait because tasks are
allocating faster than writeback can complete so I wanted to reduce the
allocation pressure. I considered waking one task at a time but there is
no prioritisation of tasks on the waitqueue and it's not clear that the
additional complexity is justified. With inaccurate counters, a light
allocator could get throttled for the full timeout unnecessarily.
Even if we were to wake one task at a time, I would prefer it was done
as a potential optimisation on top.
Diff on top based on review feedback;
diff --git a/mm/vmscan.c b/mm/vmscan.c
index bcd22e53795f..735b1f2b5d9e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1048,7 +1048,15 @@ void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
{
unsigned long nr_written;
- __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
+ inc_node_page_state(page, NR_THROTTLED_WRITTEN);
+
+ /*
+ * This is an inaccurate read as the per-cpu deltas may not
+ * be synchronised. However, given that the system is
+ * writeback throttled, it is not worth taking the penalty
+ * of getting an accurate count. At worst, the throttle
+ * timeout guarantees forward progress.
+ */
nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
READ_ONCE(pgdat->nr_reclaim_start);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/8] mm/vmscan: Throttle reclaim until some writeback completes if congested
2021-10-14 10:47 ` Mel Gorman
@ 2021-10-14 15:42 ` Vlastimil Babka
0 siblings, 0 replies; 23+ messages in thread
From: Vlastimil Babka @ 2021-10-14 15:42 UTC (permalink / raw)
To: Mel Gorman
Cc: Linux-MM, NeilBrown, Theodore Ts'o, Andreas Dilger,
Darrick J . Wong, Matthew Wilcox, Michal Hocko, Dave Chinner,
Rik van Riel, Johannes Weiner, Jonathan Corbet, Linux-fsdevel,
LKML
On 10/14/21 12:47, Mel Gorman wrote:
> Thanks Vlastimil
>
> On Wed, Oct 13, 2021 at 05:39:36PM +0200, Vlastimil Babka wrote:
>> > +/*
>> > + * Account for pages written if tasks are throttled waiting on dirty
>> > + * pages to clean. If enough pages have been cleaned since throttling
>> > + * started then wakeup the throttled tasks.
>> > + */
>> > +void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
>> > + int nr_throttled)
>> > +{
>> > + unsigned long nr_written;
>> > +
>> > + __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
>>
>> Is this intentionally using the __ version that normally expects irqs to be
>> disabled (AFAIK they are not in this path)? I think this is rarely used cold
>> path so it doesn't seem worth to trade off speed for accuracy.
>>
>
> It was intentional because IRQs can be disabled and if it's race-prone,
> it's not overly problematic but you're right, better to be safe. I changed
> it to the safe type as it's mostly free on x86, arm64 and s390 and for
> other architectures, this is a slow path.
Great, thanks.
>> > + nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
>> > + READ_ONCE(pgdat->nr_reclaim_start);
>>
>> Even if the inc above was safe, node_page_state() will return only the
>> global counter, so the value we read here will only actually increment when
>> some cpu's counter overflows, so it will be "bursty". Maybe it's ok, just
>> worth documenting?
>>
>
> I didn't think the penalty of doing an accurate read while writeback
> throttled is worth it. I'll add a comment.
>
>> > +
>> > + if (nr_written > SWAP_CLUSTER_MAX * nr_throttled)
>> > + wake_up_all(&pgdat->reclaim_wait);
>>
>> Hm it seems a bit weird that the more tasks are throttled, the more we wait,
>> and then wake up all. Theoretically this will lead to even more
>> bursty/staggering herd behavior. Could be better to wake up single task each
>> SWAP_CLUSTER_MAX, and bump nr_reclaim_start? But maybe it's not a problem in
>> practice due to HZ/10 timeouts being short enough?
>>
>
> Yes, the more tasks are throttled the longer tasks wait because tasks are
> allocating faster than writeback can complete so I wanted to reduce the
> allocation pressure. I considered waking one task at a time but there is
> no prioritisation of tasks on the waitqueue and it's not clear that the
> additional complexity is justified. With inaccurate counters, a light
> allocator could get throttled for the full timeout unnecessarily.
>
> Even if we were to wake one task at a time, I would prefer it was done
> as a potential optimisation on top.
Fair enough.
> Diff on top based on review feedback;
Thanks, with that you can add
Acked-by: Vlastimil Babka <vbabka@suse.cz>
to the updated version
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bcd22e53795f..735b1f2b5d9e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1048,7 +1048,15 @@ void __acct_reclaim_writeback(pg_data_t *pgdat, struct page *page,
> {
> unsigned long nr_written;
>
> - __inc_node_page_state(page, NR_THROTTLED_WRITTEN);
> + inc_node_page_state(page, NR_THROTTLED_WRITTEN);
> +
> + /*
> + * This is an inaccurate read as the per-cpu deltas may not
> + * be synchronised. However, given that the system is
> + * writeback throttled, it is not worth taking the penalty
> + * of getting an accurate count. At worst, the throttle
> + * timeout guarantees forward progress.
> + */
> nr_written = node_page_state(pgdat, NR_THROTTLED_WRITTEN) -
> READ_ONCE(pgdat->nr_reclaim_start);
>
^ permalink raw reply [flat|nested] 23+ messages in thread