Re: [PATCH v2 3/3] DAX: mmap write readonly file

Ross Zwisler
2017-04-11T18:52:37Z

On Tue, Apr 11, 2017 at 09:56:14AM -0400, Jeff Moyer wrote:
> Eryu Guan <eguan@redhat.com> writes:
>
> > On Mon, Apr 10, 2017 at 02:05:53PM +0800, Xiong Zhou wrote:
> >> Regression case that one can write to read-only
> >> file in a DAX mountpoint.
> >>
> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com>
> >> ---
> >>
> >> v2:
> >> compile test programme manually in this test because default
> >> cc option -O2 prevents this issue reproduction;
> >
> > Hmm, this looks.. ugly to me :) Better to find out the exact reason that
> > prevents the bug from reproducing and update the c program accordingly.
>
> The compiler probably optimizes this bit out:
>
> + /* fault in the page */
> + foo = *buf;

Yep, verified this with objdump. You can prevent the compiler from
optimizing out this bit by making 'foo' volatile. Patch at the end of this
mail, and I also verified that this works with objdump.

In my setup at least this test passes both with v4.10.0 (which does not
contain the fix we are testing for, and should fail) and with v4.10.3 (which
does contain the kernel fix, and should pass).

So, I think the test still needs a little love. :)

---

diff --git a/src/t_mmap_write_ro.c b/src/t_mmap_write_ro.c
index cce6e0d..3960815 100644
--- a/src/t_mmap_write_ro.c
+++ b/src/t_mmap_write_ro.c
@@ -11,7 +11,8 @@ int
main(int argc, char **argv)
{
int fd, pfd, ret;
- char *buf, foo;
+ char *buf;
+ volatile char foo;
int pagesize = getpagesize();

if (argc < 2) {