linux-kernel - Re: [PATCH] shmem: replace_page must flush

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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.LSU.2.00.1206140049350.5499@eggly.anvils>
Date:	Thu, 14 Jun 2012 01:20:10 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Simon Baatz <gmbnomis@...il.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Hellwig <hch@...radead.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Stephane Marchesin <marcheu@...omium.org>,
	Andi Kleen <andi@...stfloor.org>,
	Dave Airlie <airlied@...il.com>,
	Daniel Vetter <daniel@...ll.ch>,
	Rob Clark <rob.clark@...aro.org>,
	Cong Wang <xiyou.wangcong@...il.com>,
	Arnd Bergmann <arnd@...db.de>, linux-mm@...ck.org,
	linux-fsdevel@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] shmem: replace_page must flush_dcache and others

On Fri, 8 Jun 2012, Simon Baatz wrote:
> On Thu, May 31, 2012 at 03:31:27PM -0700, Hugh Dickins wrote:
> > * shmem_replace_page must flush_dcache_page after copy_highpage [akpm]
> 
> >  
> > -	*pagep = newpage;
> >  	page_cache_get(newpage);
> >  	copy_highpage(newpage, oldpage);
> > +	flush_dcache_page(newpage);
> >  
> 
> Couldn't we use the lighter flush_kernel_dcache_page() here (like in
> fs/exec.c copy_strings())?  If I got this correctly, the page is
> copied via the kernel mapping and thus, only the kernel mapping needs
> to be flushed.

Sorry for being so slow to respond, I had to focus on something else.

That's an interesting question you raise: I think you are almost right.

You are correct that it's copied via kernel mapping; and this page
cannot yet be visible to userspace.

But I have to say that you're "almost" right, because when I look up
flush_kernel_dcache_page(), I notice that it's supposed to be called
while the page is still kmapped (if kmap() or kmap_atomic() were
necessary).  So I would have to pull apart copy_highpage() and
do it inside there.

There are four uses of flush_dcache_page() in mm/shmem.c.  One of
those (in do_shmem_file_read()) should certainly not be converted
to flush_kernel_dcache_page(), but the rest could be.

However, I'm reluctant to do so myself, since I don't test on any
architecture which has a non-default flush_kernel_dcache_page(),
and I'm not at all familiar with it either.

fs/exec.c is rather an exception to be using it.  I believe it was
introduced to solve a coherency issue at the block or driver level,
and generally nothing in fs or mm has been using it.

You may well be right that savings (on arm, mips, parisc) could be
made in various places by using it in place of flush_dcache_page().
But I'd rather leave that exercise to someone who understands better
what they're doing, and can see the results if they get it wrong.

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