Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755289Ab2FNIUw (ORCPT ); Thu, 14 Jun 2012 04:20:52 -0400 Received: from mail-pz0-f46.google.com ([209.85.210.46]:59632 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754607Ab2FNIUt (ORCPT ); Thu, 14 Jun 2012 04:20:49 -0400 Date: Thu, 14 Jun 2012 01:20:10 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Simon Baatz cc: Andrew Morton , Christoph Hellwig , KAMEZAWA Hiroyuki , Alan Cox , Stephane Marchesin , Andi Kleen , Dave Airlie , Daniel Vetter , Rob Clark , Cong Wang , Arnd Bergmann , linux-mm@kvack.org, linux-fsdevel@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] shmem: replace_page must flush_dcache and others In-Reply-To: <20120608084033.GA21818@schnuecks.de> Message-ID: References: <20120608084033.GA21818@schnuecks.de> User-Agent: Alpine 2.00 (LSU 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2160 Lines: 52 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/