Development of Linux for the HP PA-RISC platform

Re: [parisc-linux] Re: Example filesystem fail to init on parisc

On Thu, 2006-03-16 at 09:13 -0700, Grant Grundler wrote:
> On Mon, Mar 13, 2006 at 12:26:34PM +0100, Miklos Szeredi wrote:
> > OK.  I'm out of ideas, CC-ing parisc-linux, maybe they can help shed
> > some light on this.
> 
> Miklos,
> I took a quick look last week but didn't see anything obvious.
> I was suspicious of the memcopy() calls to copy stuff to
> user space. But willy me told those are fine if using the
> result of kmap() (and then kunmap'ing when done).

Sorry, meant to reply earlier.

> > The reason appears to be that when the userspace filesystem reads from
> > the FUSE device, the kernel doesn't copy any data to the userspace
> > read buffer, though the correct size is returned by read().
> > 
> > FUSE uses a combination of get_user_pages(), kmap_atomic() and
> > memcpy().  After kunmap_atomic(), flush_dcache_page() is called to
> > avoid virtual aliasing.  The data in the read buffer is totally
> > untouched.

This is wrong.  A VIPT cache requires a mapping to flush on.  If you
kunmap, you've lost the mapping.  What you should do is

kmap()
operate on data
flush_kernel_dcache_page()
kunmap()

(Continue reading)

headers

Miklos Szeredi | 17 Mar 10:13

Re: [parisc-linux] Re: Example filesystem fail to init on parisc

> > > FUSE uses a combination of get_user_pages(), kmap_atomic() and
> > > memcpy().  After kunmap_atomic(), flush_dcache_page() is called to
> > > avoid virtual aliasing.  The data in the read buffer is totally
> > > untouched.
> 
> This is wrong.  A VIPT cache requires a mapping to flush on.  If you
> kunmap, you've lost the mapping.  What you should do is
> 
> kmap()
> operate on data
> flush_kernel_dcache_page()
> kunmap()

OK, this didn't help.  Looking closer it didn't have a chance of
helping, since kunmap()/kunmap_atomic() are no-ops unless
CONFIG_HIGHMEM is set, but on parisc there's no highmem support.

Any other ideas?

Thanks,
Miklos

-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642

headers

Miklos Szeredi | 16 Mar 18:31

Re: [parisc-linux] Re: Example filesystem fail to init on parisc

> This is wrong.  A VIPT cache requires a mapping to flush on.  If you
> kunmap, you've lost the mapping.

Ahh, I knew I was missing something fundamental.

> What you should do is
> 
> kmap()
> operate on data
> flush_kernel_dcache_page()
> kunmap()

Chris, can you try the below patch?

Thanks,
Miklos

Index: linux/fs/fuse/dev.c
===================================================================
--- linux.orig/fs/fuse/dev.c	2006-03-13 18:26:00.000000000 +0100
+++ linux/fs/fuse/dev.c	2006-03-16 18:28:54.000000000 +0100
@@ -432,11 +432,11 @@ static void fuse_copy_init(struct fuse_c
 static void fuse_copy_finish(struct fuse_copy_state *cs)
 {
 	if (cs->mapaddr) {
-		kunmap_atomic(cs->mapaddr, KM_USER0);
-		if (cs->write) {
+		if (cs->write)
 			flush_dcache_page(cs->pg);
+		kunmap_atomic(cs->mapaddr, KM_USER0);

(Continue reading)

headers

Chris Frost | 17 Mar 07:53

Re: [parisc-linux] Re: Example filesystem fail to init on parisc

(I'm only replying to fuse-devel@ right now since the patch doesn't quite work
and to help keep their list traffic down.)

I have applied and tested the (and only the) flush_dcache_page() patch,
with odd results to report.

null still reads zeros. And fwiw hello_ll does the same. However, our
fileserver process gets the INIT message and then errors "fuse: writing
device: Invalid argument". Several unaligned memory access are made
by our part of the process, but iirc linux catches and corrects these
with no ill correctness effects.

Attached is the strace of null (strace-null) and our kfsd (strace-kfsd).
I've included them both starting at the socketpair() and fork()
as before; strace-kfsd's fuse read activity starts at line 157.

thanks again for all this help!

On Thu, Mar 16, 2006 at 06:31:17PM +0100, Miklos Szeredi wrote:
> > This is wrong.  A VIPT cache requires a mapping to flush on.  If you
> > kunmap, you've lost the mapping.
> 
> Ahh, I knew I was missing something fundamental.
> 
> > What you should do is
> > 
> > kmap()
> > operate on data
> > flush_kernel_dcache_page()
> > kunmap()

(Continue reading)