[PATCH] fs: dax: do not build on ARC or SH

[PATCH] fs: dax: do not build on ARC or SH

[Geert Uytterhoeven]

From: Arnd Bergmann <arnd@arndb.de>

The DAX implementation relies on the architecture to provide a working
copy_user_page() function, as reported by Michael Ellerman's kisskb
build bot:

fs/dax.c: error: implicit declaration of function 'copy_user_page' [-Werror=implicit-function-declaration]:  => 266:2

We already have a list of architectures that are known to be incompatible,
but the list is missing ARC and SH at the moment. Further, blackfin and
c6x also lack support for this function, but are already excluded because
they do not support MMU-based kernels.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
[Geert: s/SH/SUPERH/, as reported by Paul Bolle <pebolle@tiscali.nl>]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 fs/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index 011f43365d7b1e53..53326a50a3ce3830 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -37,7 +37,7 @@ source "fs/f2fs/Kconfig"
 config FS_DAX
 	bool "Direct Access (DAX) support"
 	depends on MMU
-	depends on !(ARM || MIPS || SPARC)
+	depends on !(ARC || ARM || MIPS || SPARC || SUPERH)
 	help
 	  Direct Access (DAX) can be used on memory-backed block devices.
 	  If the block device supports DAX and the filesystem supports DAX,
-- 
1.9.1

Re: [PATCH] fs: dax: do not build on ARC or SH

[Boaz Harrosh]

On 07/13/2015 12:52 PM, Geert Uytterhoeven wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The DAX implementation relies on the architecture to provide a working
> copy_user_page() function, as reported by Michael Ellerman's kisskb
> build bot:
> 
> fs/dax.c: error: implicit declaration of function 'copy_user_page' [-Werror=implicit-function-declaration]:  => 266:2
> 

static int copy_user_bh(struct page *to, struct buffer_head *bh,
			unsigned blkbits, unsigned long vaddr)
{
	void *vfrom, *vto;
	if (dax_get_addr(bh, &vfrom, blkbits) < 0)
		return -EIO;
	vto = kmap_atomic(to);
	copy_user_page(vto, vfrom, vaddr, to);
	kunmap_atomic(vto);
	return 0;
}

I do not understand why we need to call copy_user_page here at all?
the destination is kmap_atomic() so it must be there right? also the
destination is the cow-to page so surly it is not yet mapped to user-space
mapping.

the from is pmem which is just there.

>From what I understand copy_user_page means:
    On these ARCHs that each user-mapping has its own VM cache, please invalidate
    the other VM caches.
Like on arm64 (arch/arm64/mm/copypage.c):
	copy_page(kto, kfrom);
	__flush_dcache_area(kto, PAGE_SIZE);

So what I do not understand is why copy_user_page does not have a default
implementation for those ARCHs that don't override it.

But again I think the above copy_user_page use is not at all needed.

And of course what do I know?

Thanks
Boaz

> We already have a list of architectures that are known to be incompatible,
> but the list is missing ARC and SH at the moment. Further, blackfin and
> c6x also lack support for this function, but are already excluded because
> they do not support MMU-based kernels.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> [Geert: s/SH/SUPERH/, as reported by Paul Bolle <pebolle@tiscali.nl>]
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  fs/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 011f43365d7b1e53..53326a50a3ce3830 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -37,7 +37,7 @@ source "fs/f2fs/Kconfig"
>  config FS_DAX
>  	bool "Direct Access (DAX) support"
>  	depends on MMU
> -	depends on !(ARM || MIPS || SPARC)
> +	depends on !(ARC || ARM || MIPS || SPARC || SUPERH)
>  	help
>  	  Direct Access (DAX) can be used on memory-backed block devices.
>  	  If the block device supports DAX and the filesystem supports DAX,
> 

Re: [PATCH] fs: dax: do not build on ARC or SH

[Matthew Wilcox]

On Mon, Jul 13, 2015 at 02:57:10PM +0300, Boaz Harrosh wrote:
> I do not understand why we need to call copy_user_page here at all?
> the destination is kmap_atomic() so it must be there right? also the
> destination is the cow-to page so surly it is not yet mapped to user-space
> mapping.
> 
> the from is pmem which is just there.
> 
> >From what I understand copy_user_page means:
>     On these ARCHs that each user-mapping has its own VM cache, please invalidate
>     the other VM caches.
> Like on arm64 (arch/arm64/mm/copypage.c):
> 	copy_page(kto, kfrom);
> 	__flush_dcache_area(kto, PAGE_SIZE);

You're confusing implementation with guaranteed semantics.  The problem
is for architectures which have virtually indexed caches, the kernel
virtual address does not necessarily map to the same cacheline as user
virtual addresses.  The solution that has been adopted for page cache
pages is that user addresses are flushed before the kernel reads from a
page, and kernel addresses are flushed before the kernel writes to a page.

Now, imagine task A mmaps a file using MAP_SHARED.  Task B mmaps the
same file using MAP_PRIVATE.  Task A & B have a communication channel,
maybe a socket.  Task A stores a few bytes to a page in the mmap, and
then sends a message down the communication channel.  Task B stores a
byte to a different part of the same page (causing the COW) and then
examines the bytes that task A wrote.

To avoid violating causality, we must have copied the bytes that task
B would have seen at that time, as opposed to the bytes which were in
storage before task A overwrote them.  So either we flush the bytes that
task A wrote before doing the COW, or we copy from an address that is
cache-coherent with the address that A used to do the store.

> So what I do not understand is why copy_user_page does not have a default
> implementation for those ARCHs that don't override it.

copy_user_page() is a documented part of the cache flushing protocols.
Some architectures have chosen to not implement it, even though they
actually need the flushing.

There is a separate issue which is that DAX is not currently doing enough
flushing.  Fixing that is about fourth on my priority list right now.
2MB pages, RDMA access and a rather interesting bug reported to me last
week are all higher priority.