linux-kernel - RE: [PATCH 1/14] mm: invalidate_mapping

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: <ba74b4e5-500e-4662-ade0-c0b714b8f570@default>
Date:	Tue, 31 May 2011 08:49:02 -0700 (PDT)
From:	Dan Magenheimer <dan.magenheimer@...cle.com>
To:	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	chris.mason@...cle.com
Subject: RE: [PATCH 1/14] mm: invalidate_mapping_pages flush cleancache

> From: Hugh Dickins [mailto:hughd@...gle.com]
> Sent: Monday, May 30, 2011 6:36 PM
> To: Andrew Morton
> Cc: Dan Magenheimer; linux-kernel@...r.kernel.org; linux-mm@...ck.org
> Subject: [PATCH 1/14] mm: invalidate_mapping_pages flush cleancache
> 
> truncate_inode_pages_range() and invalidate_inode_pages2_range()
> call cleancache_flush_inode(mapping) before and after: shouldn't
> invalidate_mapping_pages() be doing the same?
> 
> Signed-off-by: Hugh Dickins <hughd@...gle.com>
> Cc: Dan Magenheimer <dan.magenheimer@...cle.com>
> ---
>  mm/truncate.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- linux.orig/mm/truncate.c	2011-05-30 13:56:10.416798124 -0700
> +++ linux/mm/truncate.c	2011-05-30 14:08:46.612547848 -0700
> @@ -333,6 +333,7 @@ unsigned long invalidate_mapping_pages(s
>  	unsigned long count = 0;
>  	int i;
> 
> +	cleancache_flush_inode(mapping);
>  	pagevec_init(&pvec, 0);
>  	while (next <= end &&
>  			pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {
> @@ -373,6 +374,7 @@ unsigned long invalidate_mapping_pages(s
>  		mem_cgroup_uncharge_end();
>  		cond_resched();
>  	}
> +	cleancache_flush_inode(mapping);
>  	return count;
>  }
>  EXPORT_SYMBOL(invalidate_mapping_pages);

Hi Hugh --

I don't claim to be an expert on VFS, and so I have cc'ed
Chris Mason who originally placed the cleancache hooks
in VFS, but I think this patch is unnecessary.  Instead
of flushing ALL of the cleancache pages belonging to
the inode with cleancache_flush_inode, the existing code
eventually calls __delete_from_page_cache on EACH page
that is being invalidated. And since __delete_from_page_cache
calls cleancache_flush_page, only that subset of pages
in the mapping that invalidate_mapping_pages() would
invalidate (which, from the comment above the routine
indicates, is only *unlocked* pages) is removed from
cleancache.

However, there may be some path through VFS I am missing
or something else subtle I am misunderstanding so please
clarify if either is true.

Thanks,
Dan
---
Thanks... for the memory!
I really could use more / my throughput's on the floor
The balloon is flat / my swap disk's fat / I've OOM's in store
Overcommitted so much
(with apologies to Bob Hope)

--
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/