| Message ID | 20210505150628.111735-75-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Memory folios | expand |
Hi "Matthew,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20210505]
[cannot apply to hnaz-linux-mm/master xfs-linux/for-next tip/perf/core shaggy/jfs-next block/for-next linus/master asm-generic/master v5.12 v5.12-rc8 v5.12-rc7 v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Memory-folios/20210506-014108
base: 29955e0289b3255c5f609a7564a0f0bb4ae35c7a
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b1883a3797e1623bf783141c25482fee16e1031c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Memory-folios/20210506-014108
git checkout b1883a3797e1623bf783141c25482fee16e1031c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from mm/workingset.c:8:
include/linux/memcontrol.h: In function 'folio_uncharge_cgroup':
include/linux/memcontrol.h:1213:42: error: parameter name omitted
1213 | static inline void folio_uncharge_cgroup(struct folio *)
| ^~~~~~~~~~~~~~
mm/workingset.c: In function 'unpack_shadow':
mm/workingset.c:201:15: warning: variable 'nid' set but not used [-Wunused-but-set-variable]
201 | int memcgid, nid;
| ^~~
mm/workingset.c: In function 'workingset_refault':
>> mm/workingset.c:348:10: error: implicit declaration of function 'folio_memcg' [-Werror=implicit-function-declaration]
348 | memcg = folio_memcg(folio);
| ^~~~~~~~~~~
>> mm/workingset.c:348:8: warning: assignment to 'struct mem_cgroup *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
348 | memcg = folio_memcg(folio);
| ^
cc1: some warnings being treated as errors
vim +/folio_memcg +348 mm/workingset.c
272
273 /**
274 * workingset_refault - evaluate the refault of a previously evicted folio
275 * @page: the freshly allocated replacement folio
276 * @shadow: shadow entry of the evicted folio
277 *
278 * Calculates and evaluates the refault distance of the previously
279 * evicted folio in the context of the node and the memcg whose memory
280 * pressure caused the eviction.
281 */
282 void workingset_refault(struct folio *folio, void *shadow)
283 {
284 bool file = folio_is_file_lru(folio);
285 struct mem_cgroup *eviction_memcg;
286 struct lruvec *eviction_lruvec;
287 unsigned long refault_distance;
288 unsigned long workingset_size;
289 struct pglist_data *pgdat;
290 struct mem_cgroup *memcg;
291 unsigned long eviction;
292 struct lruvec *lruvec;
293 unsigned long refault;
294 bool workingset;
295 int memcgid;
296
297 unpack_shadow(shadow, &memcgid, &pgdat, &eviction, &workingset);
298
299 rcu_read_lock();
300 /*
301 * Look up the memcg associated with the stored ID. It might
302 * have been deleted since the folio's eviction.
303 *
304 * Note that in rare events the ID could have been recycled
305 * for a new cgroup that refaults a shared folio. This is
306 * impossible to tell from the available data. However, this
307 * should be a rare and limited disturbance, and activations
308 * are always speculative anyway. Ultimately, it's the aging
309 * algorithm's job to shake out the minimum access frequency
310 * for the active cache.
311 *
312 * XXX: On !CONFIG_MEMCG, this will always return NULL; it
313 * would be better if the root_mem_cgroup existed in all
314 * configurations instead.
315 */
316 eviction_memcg = mem_cgroup_from_id(memcgid);
317 if (!mem_cgroup_disabled() && !eviction_memcg)
318 goto out;
319 eviction_lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat);
320 refault = atomic_long_read(&eviction_lruvec->nonresident_age);
321
322 /*
323 * Calculate the refault distance
324 *
325 * The unsigned subtraction here gives an accurate distance
326 * across nonresident_age overflows in most cases. There is a
327 * special case: usually, shadow entries have a short lifetime
328 * and are either refaulted or reclaimed along with the inode
329 * before they get too old. But it is not impossible for the
330 * nonresident_age to lap a shadow entry in the field, which
331 * can then result in a false small refault distance, leading
332 * to a false activation should this old entry actually
333 * refault again. However, earlier kernels used to deactivate
334 * unconditionally with *every* reclaim invocation for the
335 * longest time, so the occasional inappropriate activation
336 * leading to pressure on the active list is not a problem.
337 */
338 refault_distance = (refault - eviction) & EVICTION_MASK;
339
340 /*
341 * The activation decision for this folio is made at the level
342 * where the eviction occurred, as that is where the LRU order
343 * during folio reclaim is being determined.
344 *
345 * However, the cgroup that will own the folio is the one that
346 * is actually experiencing the refault event.
347 */
> 348 memcg = folio_memcg(folio);
349 lruvec = mem_cgroup_lruvec(memcg, pgdat);
350
351 inc_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file);
352
353 /*
354 * Compare the distance to the existing workingset size. We
355 * don't activate pages that couldn't stay resident even if
356 * all the memory was available to the workingset. Whether
357 * workingset competition needs to consider anon or not depends
358 * on having swap.
359 */
360 workingset_size = lruvec_page_state(eviction_lruvec, NR_ACTIVE_FILE);
361 if (!file) {
362 workingset_size += lruvec_page_state(eviction_lruvec,
363 NR_INACTIVE_FILE);
364 }
365 if (mem_cgroup_get_nr_swap_pages(memcg) > 0) {
366 workingset_size += lruvec_page_state(eviction_lruvec,
367 NR_ACTIVE_ANON);
368 if (file) {
369 workingset_size += lruvec_page_state(eviction_lruvec,
370 NR_INACTIVE_ANON);
371 }
372 }
373 if (refault_distance > workingset_size)
374 goto out;
375
376 folio_set_active_flag(folio);
377 workingset_age_nonresident(lruvec, folio_nr_pages(folio));
378 inc_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + file);
379
380 /* Folio was active prior to eviction */
381 if (workingset) {
382 folio_set_workingset_flag(folio);
383 /* XXX: Move to lru_cache_add() when it supports new vs putback */
384 lru_note_cost_folio(folio);
385 inc_lruvec_state(lruvec, WORKINGSET_RESTORE_BASE + file);
386 }
387 out:
388 rcu_read_unlock();
389 }
390
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, May 06, 2021 at 04:17:27AM +0800, kernel test robot wrote: > In file included from mm/workingset.c:8: > include/linux/memcontrol.h: In function 'folio_uncharge_cgroup': > include/linux/memcontrol.h:1213:42: error: parameter name omitted > 1213 | static inline void folio_uncharge_cgroup(struct folio *) > | ^~~~~~~~~~~~~~ Fixed (also reported in your other report) > mm/workingset.c: In function 'unpack_shadow': > mm/workingset.c:201:15: warning: variable 'nid' set but not used [-Wunused-but-set-variable] > 201 | int memcgid, nid; > | ^~~ I didn't introduce this one; not trying to fix it ;-) > mm/workingset.c: In function 'workingset_refault': > >> mm/workingset.c:348:10: error: implicit declaration of function 'folio_memcg' [-Werror=implicit-function-declaration] > 348 | memcg = folio_memcg(folio); > | ^~~~~~~~~~~ > >> mm/workingset.c:348:8: warning: assignment to 'struct mem_cgroup *' from 'int' makes pointer from integer without a cast [-Wint-conversion] > 348 | memcg = folio_memcg(folio); > | ^ > cc1: some warnings being treated as errors Fixed. Thanks!
diff --git a/include/linux/swap.h b/include/linux/swap.h index d1cb67cdb476..35d3dba422a8 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -323,7 +323,7 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio) /* linux/mm/workingset.c */ void workingset_age_nonresident(struct lruvec *lruvec, unsigned long nr_pages); void *workingset_eviction(struct page *page, struct mem_cgroup *target_memcg); -void workingset_refault(struct page *page, void *shadow); +void workingset_refault(struct folio *folio, void *shadow); void workingset_activation(struct folio *folio); /* Only track the nodes of mappings with shadow entries */ @@ -344,7 +344,7 @@ extern unsigned long nr_free_buffer_pages(void); /* linux/mm/swap.c */ extern void lru_note_cost(struct lruvec *lruvec, bool file, unsigned int nr_pages); -extern void lru_note_cost_page(struct page *); +extern void lru_note_cost_folio(struct folio *); extern void lru_cache_add(struct page *); void mark_page_accessed(struct page *); void folio_mark_accessed(struct folio *); diff --git a/mm/filemap.c b/mm/filemap.c index e6aa49e32255..5c130bfcdb1c 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -979,7 +979,7 @@ int add_to_page_cache_lru(struct page *page, struct address_space *mapping, */ WARN_ON_ONCE(PageActive(page)); if (!(gfp_mask & __GFP_WRITE) && shadow) - workingset_refault(page, shadow); + workingset_refault(page_folio(page), shadow); lru_cache_add(page); } return ret; diff --git a/mm/memory.c b/mm/memory.c index fc3f50d0702c..a73da89c36ef 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3364,7 +3364,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) shadow = get_shadow_from_swap_cache(entry); if (shadow) - workingset_refault(page, shadow); + workingset_refault(page_folio(page), + shadow); lru_cache_add(page); diff --git a/mm/swap.c b/mm/swap.c index 8e7f92be2f6f..cd441cdb82fd 100644 --- a/mm/swap.c +++ b/mm/swap.c @@ -311,10 +311,10 @@ void lru_note_cost(struct lruvec *lruvec, bool file, unsigned int nr_pages) } while ((lruvec = parent_lruvec(lruvec))); } -void lru_note_cost_page(struct page *page) +void lru_note_cost_folio(struct folio *folio) { - lru_note_cost(mem_cgroup_page_lruvec(page, page_pgdat(page)), - page_is_file_lru(page), thp_nr_pages(page)); + lru_note_cost(mem_cgroup_folio_lruvec(folio, folio_pgdat(folio)), + folio_is_file_lru(folio), folio_nr_pages(folio)); } static void __activate_page(struct page *page, struct lruvec *lruvec) diff --git a/mm/swap_state.c b/mm/swap_state.c index 272ea2108c9d..1c8e8b3aa10b 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -503,7 +503,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, mem_cgroup_swapin_uncharge_swap(entry); if (shadow) - workingset_refault(page, shadow); + workingset_refault(page_folio(page), shadow); /* Caller will initiate read into locked page */ lru_cache_add(page); diff --git a/mm/workingset.c b/mm/workingset.c index d969403f2b2a..93002e4f8f15 100644 --- a/mm/workingset.c +++ b/mm/workingset.c @@ -271,17 +271,17 @@ void *workingset_eviction(struct page *page, struct mem_cgroup *target_memcg) } /** - * workingset_refault - evaluate the refault of a previously evicted page - * @page: the freshly allocated replacement page - * @shadow: shadow entry of the evicted page + * workingset_refault - evaluate the refault of a previously evicted folio + * @page: the freshly allocated replacement folio + * @shadow: shadow entry of the evicted folio * * Calculates and evaluates the refault distance of the previously - * evicted page in the context of the node and the memcg whose memory + * evicted folio in the context of the node and the memcg whose memory * pressure caused the eviction. */ -void workingset_refault(struct page *page, void *shadow) +void workingset_refault(struct folio *folio, void *shadow) { - bool file = page_is_file_lru(page); + bool file = folio_is_file_lru(folio); struct mem_cgroup *eviction_memcg; struct lruvec *eviction_lruvec; unsigned long refault_distance; @@ -299,10 +299,10 @@ void workingset_refault(struct page *page, void *shadow) rcu_read_lock(); /* * Look up the memcg associated with the stored ID. It might - * have been deleted since the page's eviction. + * have been deleted since the folio's eviction. * * Note that in rare events the ID could have been recycled - * for a new cgroup that refaults a shared page. This is + * for a new cgroup that refaults a shared folio. This is * impossible to tell from the available data. However, this * should be a rare and limited disturbance, and activations * are always speculative anyway. Ultimately, it's the aging @@ -338,14 +338,14 @@ void workingset_refault(struct page *page, void *shadow) refault_distance = (refault - eviction) & EVICTION_MASK; /* - * The activation decision for this page is made at the level + * The activation decision for this folio is made at the level * where the eviction occurred, as that is where the LRU order - * during page reclaim is being determined. + * during folio reclaim is being determined. * - * However, the cgroup that will own the page is the one that + * However, the cgroup that will own the folio is the one that * is actually experiencing the refault event. */ - memcg = page_memcg(page); + memcg = folio_memcg(folio); lruvec = mem_cgroup_lruvec(memcg, pgdat); inc_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file); @@ -373,15 +373,15 @@ void workingset_refault(struct page *page, void *shadow) if (refault_distance > workingset_size) goto out; - SetPageActive(page); - workingset_age_nonresident(lruvec, thp_nr_pages(page)); + folio_set_active_flag(folio); + workingset_age_nonresident(lruvec, folio_nr_pages(folio)); inc_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + file); - /* Page was active prior to eviction */ + /* Folio was active prior to eviction */ if (workingset) { - SetPageWorkingset(page); + folio_set_workingset_flag(folio); /* XXX: Move to lru_cache_add() when it supports new vs putback */ - lru_note_cost_page(page); + lru_note_cost_folio(folio); inc_lruvec_state(lruvec, WORKINGSET_RESTORE_BASE + file); } out:
This nets us 178 bytes of savings from removing calls to compound_head. The three callers all grow a little, but each of them will be converted to use folios soon, so that's fine. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/swap.h | 4 ++-- mm/filemap.c | 2 +- mm/memory.c | 3 ++- mm/swap.c | 6 +++--- mm/swap_state.c | 2 +- mm/workingset.c | 34 +++++++++++++++++----------------- 6 files changed, 26 insertions(+), 25 deletions(-)