linux-kernel - Re: [PATCH 3/3] mm/fs: fix pessimization in hole-punching pagecache

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: <53B55450.3060908@suse.cz>
Date:	Thu, 03 Jul 2014 15:02:08 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
CC:	Sasha Levin <sasha.levin@...cle.com>,
	Konstantin Khlebnikov <koct9i@...il.com>,
	Lukas Czerner <lczerner@...hat.com>,
	Dave Jones <davej@...hat.com>, linux-mm@...ck.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] mm/fs: fix pessimization in hole-punching pagecache

On 07/02/2014 09:13 PM, Hugh Dickins wrote:
> I wanted to revert my v3.1 commit d0823576bf4b ("mm: pincer in
> truncate_inode_pages_range"), to keep truncate_inode_pages_range()
> in synch with shmem_undo_range(); but have stepped back - a change
> to hole-punching in truncate_inode_pages_range() is a change to
> hole-punching in every filesystem (except tmpfs) that supports it.
>
> If there's a logical proof why no filesystem can depend for its own
> correctness on the pincer guarantee in truncate_inode_pages_range() -
> an instant when the entire hole is removed from pagecache - then let's
> revisit later.  But the evidence is that only tmpfs suffered from the
> livelock, and we have no intention of extending hole-punch to ramfs.
> So for now just add a few comments (to match or differ from those in
> shmem_undo_range()), and fix one silliness noticed in d0823576bf4b...
>
> Its "index == start" addition to the hole-punch termination test was
> incomplete: it opened a way for the end condition to be missed, and the
> loop go on looking through the radix_tree, all the way to end of file.
> Fix that pessimization by resetting index when detected in inner loop.
>
> Note that it's actually hard to hit this case, without the obsessive
> concurrent faulting that trinity does: normally all pages are removed
> in the initial trylock_page() pass, and this loop finds nothing to do.
> I had to "#if 0" out the initial pass to reproduce bug and test fix.
>
> Signed-off-by: Hugh Dickins <hughd@...gle.com>
> Cc: Sasha Levin <sasha.levin@...cle.com>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

> Cc: Konstantin Khlebnikov <koct9i@...il.com>
> Cc: Lukas Czerner <lczerner@...hat.com>
> Cc: Dave Jones <davej@...hat.com>
> ---
>
>   mm/truncate.c |   11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> --- 3.16-rc3/mm/truncate.c	2014-06-08 11:19:54.000000000 -0700
> +++ linux/mm/truncate.c	2014-07-02 03:36:05.972553533 -0700
> @@ -355,14 +355,16 @@ void truncate_inode_pages_range(struct a
>   	for ( ; ; ) {
>   		cond_resched();
>   		if (!pagevec_lookup_entries(&pvec, mapping, index,
> -			min(end - index, (pgoff_t)PAGEVEC_SIZE),
> -			indices)) {
> +			min(end - index, (pgoff_t)PAGEVEC_SIZE), indices)) {
> +			/* If all gone from start onwards, we're done */
>   			if (index == start)
>   				break;
> +			/* Otherwise restart to make sure all gone */
>   			index = start;
>   			continue;
>   		}
>   		if (index == start && indices[0] >= end) {
> +			/* All gone out of hole to be punched, we're done */
>   			pagevec_remove_exceptionals(&pvec);
>   			pagevec_release(&pvec);
>   			break;
> @@ -373,8 +375,11 @@ void truncate_inode_pages_range(struct a
>
>   			/* We rely upon deletion not changing page->index */
>   			index = indices[i];
> -			if (index >= end)
> +			if (index >= end) {
> +				/* Restart punch to make sure all gone */
> +				index = start - 1;
>   				break;
> +			}
>
>   			if (radix_tree_exceptional_entry(page)) {
>   				clear_exceptional_entry(mapping, index, page);
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/