RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

[David Woodhouse]

Why does a _readonly_ mapping fail if the file system has no writepage 
method? 

do_mmap_pgoff() sets VM_MAYWRITE on the vma and then generic_file_mmap() 
refuses to allow it. 

Suggested patch below.... or should I just hack fsx-linux to use 
MAP_PRIVATE for its readonly mappings and ignore it?

--- 1.157/mm/filemap.c  Sun Nov  3 02:55:27 2002
+++ edited/mm/filemap.c Fri Nov  8 22:08:22 2002
@@ -1311,9 +1311,12 @@
        struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
        struct inode *inode = mapping->host;

-       if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE)) {
-               if (!mapping->a_ops->writepage)
+       if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE) &&
+           !mapping->a_ops->writepage) {
+               if (vma->vm_flags & VM_WRITE)
                        return -EINVAL;
+               else
+                       vma->vm_flags &= ~VM_MAYWRITE;
        }
        if (!mapping->a_ops->readpage)
                return -ENOEXEC;


--
dwmw2

Re: RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

[Linus Torvalds]

In article <24305.1036795742@passion.cambridge.redhat.com>,
David Woodhouse  <dwmw2@infradead.org> wrote:
>Why does a _readonly_ mapping fail if the file system has no writepage 
>method? 
>
>do_mmap_pgoff() sets VM_MAYWRITE on the vma and then generic_file_mmap() 
>refuses to allow it. 
>
>Suggested patch below.... or should I just hack fsx-linux to use 
>MAP_PRIVATE for its readonly mappings and ignore it?

This is broken. Since it has VM_MAYWRITE, a subsequent mprotect() may
mark it writable, and you you went boom.

If you really want a shared mapping, you'd better open with O_RDONLY, at
which point the existing code should be perfectly happy and does the
right thing.

In other words: the code is correct as-is.

		Linus

Re: RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

[David Woodhouse]

torvalds@transmeta.com said:
>  This is broken. Since it has VM_MAYWRITE, a subsequent mprotect() may
> mark it writable, and you you went boom.

Er, we clear VM_MAYWRITE...

+               if (vma->vm_flags & VM_WRITE)
                        return -EINVAL;
+               else
+                       vma->vm_flags &= ~VM_MAYWRITE;

> If you really want a shared mapping, you'd better open with O_RDONLY,
> at which point the existing code should be perfectly happy and does
> the right thing. 

It's a read-only mapping. Whether it's shared or private is not relevant,
surely, since those affect only the behaviour if we write to it -- which we 
can't. 

I don't _really_ want a shared mapping; all I want is for the fsx-linux
stress test to run, and find interesting breakage on my file system to keep
me from getting bored (what are Friday nights for, after all?).

As shipped, fsx-linux uses PROT_READ, MAP_SHARED on its test file, which
definitely needs to be opened for write. For now, I've just changed it to
use MAP_PRIVATE. I'm just a bit concerned about having to change the test to
get it to work though, and don't see why a _readonly_ mmap should fail due 
to lack of writepage.

--
dwmw2

Re: RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

[Linus Torvalds]

On Fri, 8 Nov 2002, David Woodhouse wrote:
> 
> It's a read-only mapping. Whether it's shared or private is not relevant,
> surely, since those affect only the behaviour if we write to it -- which we 
> can't. 

But why do you expect to be able to mmap shared something you opened RW in
the first place, if the filesystem cannot handle writable m appings?

> I don't _really_ want a shared mapping; all I want is for the fsx-linux
> stress test to run, and find interesting breakage on my file system to keep
> me from getting bored (what are Friday nights for, after all?).

Ok, it's good to have tests, but it sounds like the test is broken.

> As shipped, fsx-linux uses PROT_READ, MAP_SHARED on its test file

Ad shipped, the kernel doesn't allow that.

Notice the pattern here? "As shipped". You have two choices: make 
gratuitous changes to the kernel to make some random test happy, or fix 
the test.

I think you should fix the test. The kernel change buys you _zero_ new 
features.

		Linus

Re: RFC: mmap(PROT_READ, MAP_SHARED) fails if !writepage.

[David Woodhouse]

dwmw2@infradead.org said:
> Suggested patch below.... or should I just hack fsx-linux to use
> MAP_PRIVATE for its readonly mappings and ignore it? 


torvalds@transmeta.com said:
>  You have two choices: make  gratuitous changes to the kernel to make
> some random test happy, or fix  the test.

> I think you should fix the test. The kernel change buys you _zero_ new
>  features.

Works for me. Thanks for the answer.

--
dwmw2