linux-kernel - Re: [PATCHv2 05/11] mm/truncate: Use folio_set_dropbehind() instead of deactivate_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4gqJqcO8wau0sgN@casper.infradead.org>
Date: Wed, 15 Jan 2025 21:35:34 +0000
From: Matthew Wilcox <willy@...radead.org>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Jens Axboe <axboe@...nel.dk>,
	"Jason A. Donenfeld" <Jason@...c4.com>,
	Andi Shyti <andi.shyti@...ux.intel.com>,
	Chengming Zhou <chengming.zhou@...ux.dev>,
	Christian Brauner <brauner@...nel.org>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Dan Carpenter <dan.carpenter@...aro.org>,
	David Airlie <airlied@...il.com>,
	David Hildenbrand <david@...hat.com>, Hao Ge <gehao@...inos.cn>,
	Jani Nikula <jani.nikula@...ux.intel.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
	Josef Bacik <josef@...icpanda.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Miklos Szeredi <miklos@...redi.hu>, Nhat Pham <nphamcs@...il.com>,
	Oscar Salvador <osalvador@...e.de>,
	Ran Xiaokai <ran.xiaokai@....com.cn>,
	Rodrigo Vivi <rodrigo.vivi@...el.com>,
	Simona Vetter <simona@...ll.ch>,
	Steven Rostedt <rostedt@...dmis.org>,
	Tvrtko Ursulin <tursulin@...ulin.net>,
	Vlastimil Babka <vbabka@...e.cz>,
	Yosry Ahmed <yosryahmed@...gle.com>, Yu Zhao <yuzhao@...gle.com>,
	intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-mm@...ck.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCHv2 05/11] mm/truncate: Use folio_set_dropbehind() instead
 of deactivate_file_folio()

On Wed, Jan 15, 2025 at 11:31:29AM +0200, Kirill A. Shutemov wrote:
> -static void lru_deactivate_file(struct lruvec *lruvec, struct folio *folio)
> -{
> -	bool active = folio_test_active(folio) || lru_gen_enabled();
> -	long nr_pages = folio_nr_pages(folio);
> -
> -	if (folio_test_unevictable(folio))
> -		return;
> -
> -	/* Some processes are using the folio */
> -	if (folio_mapped(folio))
> -		return;
> -
> -	lruvec_del_folio(lruvec, folio);
> -	folio_clear_active(folio);
> -	folio_clear_referenced(folio);
> -
> -	if (folio_test_writeback(folio) || folio_test_dirty(folio)) {
> -		/*
> -		 * Setting the reclaim flag could race with
> -		 * folio_end_writeback() and confuse readahead.  But the
> -		 * race window is _really_ small and  it's not a critical
> -		 * problem.
> -		 */
> -		lruvec_add_folio(lruvec, folio);
> -		folio_set_reclaim(folio);
> -	} else {
> -		/*
> -		 * The folio's writeback ended while it was in the batch.
> -		 * We move that folio to the tail of the inactive list.
> -		 */
> -		lruvec_add_folio_tail(lruvec, folio);
> -		__count_vm_events(PGROTATED, nr_pages);
> -	}
> -
> -	if (active) {
> -		__count_vm_events(PGDEACTIVATE, nr_pages);
> -		__count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE,
> -				     nr_pages);
> -	}
> -}

> +++ b/mm/truncate.c
> @@ -486,7 +486,7 @@ unsigned long mapping_try_invalidate(struct address_space *mapping,
>  			 * of interest and try to speed up its reclaim.
>  			 */
>  			if (!ret) {
> -				deactivate_file_folio(folio);
> +				folio_set_dropbehind(folio);

brr.

This is a fairly substantial change in semantics, and maybe it's fine.

At a high level, we're trying to remove pages from an inode that aren't
in use.  But we might find that some of them are in use (eg they're
mapped or under writeback).  If they are mapped, we don't currently
try to accelerate their reclaim, but now we're going to mark them
as dropbehind.  I think that's wrong.

If they're dirty or under writeback, then yes, mark them as dropbehind, but
I think we need to be a little more surgical here.  Maybe preserve the
unevictable check too.