[PATCH 0/3] DAX: Fix mmap-write not updating c/mtime

[PATCH 0/3] DAX: Fix mmap-write not updating c/mtime

[Boaz Harrosh]

Hi

First submitted an xfstest (generic/080) that demonstrates our current problem.

   When run on ext4-dax mount, and I'd imagine Dave's xfs-dax mount this test
   fails, on regular mount it works just fine.

then 2 patches to fix it.

The main problem is that current mm/memory.c will no call us with page_mkwrite
if we do not have an actual page mapping, which is what DAX uses.
The solution presented here introduces a new pfn_mkwrite to solve this problem.
Please see patch-2 for details.

I've been running with this patch for 4 month both HW and VMs with no apparent
danger, but see patch-2 I played it safe.

List of patches:
[PATCH 1/2] generic: 080 test that mmap-write updates c/mtime

	This one is for Dave Chinner to submit to xfstests

[PATCH 1/2] mm: New pfn_mkwrite same as page_mkwrite for VM_PFNMAP

	This patch I need help with please look into it?

[PATCH 2/2] dax: use pfn_mkwrite to update mtime

Please review and comment.

Thanks
Boaz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/3] xfstests: generic/080 test that mmap-write updates c/mtime

[Boaz Harrosh]

From: Yigal Korman <yigal@plexistor.com>

when using mmap() for file i/o, writing to the file should update
it's c/mtime. Specifically if we first mmap-read from a page, then
memap-write to the same page.

This test was failing for the initial submission of DAX because
pfn based mapping do not have an page_mkwrite called for them.
The new Kernel patches that introduce pfn_mkwrite fixes this test.

Signed-off-by: Yigal Korman <yigal@plexistor.com>
Signed-off-by: Omer Zilberberg <omzg@plexistor.com>
Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 .gitignore            |   1 +
 src/Makefile          |   2 +-
 src/mmap_mtime.c      | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/080     |  53 ++++++++++++++++++++++
 tests/generic/080.out |   4 ++
 tests/generic/group   |   1 +
 6 files changed, 182 insertions(+), 1 deletion(-)
 create mode 100644 src/mmap_mtime.c
 create mode 100644 tests/generic/080
 create mode 100644 tests/generic/080.out

diff --git a/.gitignore b/.gitignore
index 41e1dc4..fd71526 100644
--- a/.gitignore
+++ b/.gitignore
@@ -119,6 +119,7 @@
 /src/cloner
 /src/renameat2
 /src/t_rename_overwrite
+/src/mmap_mtime
 
 # dmapi/ binaries
 /dmapi/src/common/cmd/read_invis
diff --git a/src/Makefile b/src/Makefile
index fa5f0f4..0e48728 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,7 +19,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	bulkstat_unlink_test_modified t_dir_offset t_futimens t_immutable \
 	stale_handle pwrite_mmap_blocked t_dir_offset2 seek_sanity_test \
 	seek_copy_test t_readdir_1 t_readdir_2 fsync-tester nsexec cloner \
-	renameat2 t_getcwd e4compact
+	renameat2 t_getcwd e4compact mmap_mtime
 
 SUBDIRS =
 
diff --git a/src/mmap_mtime.c b/src/mmap_mtime.c
new file mode 100644
index 0000000..9a83227
--- /dev/null
+++ b/src/mmap_mtime.c
@@ -0,0 +1,122 @@
+/*
+ * test to check that mtime is updated when writing to mmap
+ *
+ * Copyright (c) 2014 Plexistor Ltd. All rights reserved.
+*/
+
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <time.h>
+#include <sys/mman.h>
+#include <sys/signal.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <stdint.h>
+
+#define EXIT_SUCCESS 0
+#define EXIT_ERROR 1
+#define EXIT_TEST_FAILED 2
+
+#define PAGE_SIZE (getpagesize())
+
+struct timespec get_mtime(int fd)
+{
+	struct stat st;
+	int ret;
+
+	ret = fstat(fd, &st);
+	if (ret) {
+		perror("fstat");
+		exit(EXIT_TEST_FAILED);
+	}
+
+	/*
+	printf("%d mtime: %lld.%.9ld\n", fd,
+		(long long)st.st_mtim.tv_sec, st.st_mtim.tv_nsec);
+	*/
+
+	return st.st_mtim;
+}
+
+void print_usage (const char* progname)
+{
+	fprintf (stderr, "%s <filename>\n", progname);
+	exit(EXIT_ERROR);
+}
+
+int main(int argc, char *argv[])
+{
+	int ret = EXIT_SUCCESS;
+	loff_t size;
+	const char* filename;
+	int fd;
+	void *mapped_mem;
+	int i;
+	struct timespec before, after;
+	long tempc = 0;
+	uint64_t tempbuf[PAGE_SIZE/sizeof(uint64_t)]; // 4K buf
+
+	if (argc < 2)
+		print_usage(argv[0]);
+
+	filename = argv[1];
+	size = PAGE_SIZE;
+
+	fd = open(filename, O_CREAT | O_EXCL | O_RDWR, 0666);
+	if (fd < 0) {
+		fprintf(stderr, "%s: Cannot open `%s': %s\n",
+			argv[0], filename, strerror(errno));
+		exit(EXIT_ERROR);
+	}
+
+	// fill the file with random data in order to make sure we
+	// won't get fake "zero" pages from FS
+	if (write(fd, tempbuf, size) < 0) {
+		perror("write");
+		close(fd);
+		exit(EXIT_ERROR);
+	}
+
+	mapped_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	if (mapped_mem == MAP_FAILED) {
+		perror("mmap");
+		close(fd);
+		exit(EXIT_TEST_FAILED);
+	}
+
+	printf("reading page...");
+	for (i = 0; i < size/sizeof(char); i++) {
+		char *p = ((char*)mapped_mem) + i;
+		tempc += *p;
+	}
+	printf("done\n");
+
+	before = get_mtime(fd);
+	sleep(1);
+
+	printf("writing something...");
+	for (i = 0; i < size/sizeof(char); i++) {
+		char *p = ((char*)mapped_mem) + i;
+		*p = tempc + i;
+	}
+	printf("done\n");
+
+	after = get_mtime(fd);
+
+	if ((before.tv_sec == after.tv_sec) &&
+			(before.tv_nsec == after.tv_nsec)) {
+		printf("Failure. mtime was not updated.\n");
+		ret = EXIT_TEST_FAILED;
+	} else {
+		// assuming no time travel/warp
+		printf("Success. mtime was updated as expected\n");
+	}
+
+	munmap(mapped_mem,0);
+	close(fd);
+	exit(ret);
+}
diff --git a/tests/generic/080 b/tests/generic/080
new file mode 100644
index 0000000..42e2a49
--- /dev/null
+++ b/tests/generic/080
@@ -0,0 +1,53 @@
+#! /bin/bash
+# FS QA Test No. 080
+#
+# Verify that mtime is updated when writing to mmap-ed pages
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Yigal Korman (yigal@plexistor.com).  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+    rm -f $TEST_DIR/mmap_mtime_testfile
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os IRIX Linux
+_require_test
+
+$here/src/mmap_mtime $TEST_DIR/mmap_mtime_testfile
+status=$?
+exit
diff --git a/tests/generic/080.out b/tests/generic/080.out
new file mode 100644
index 0000000..118fd24
--- /dev/null
+++ b/tests/generic/080.out
@@ -0,0 +1,4 @@
+QA output created by 080
+reading page...done
+writing something...done
+Success. mtime was updated as expected
diff --git a/tests/generic/group b/tests/generic/group
index 11ce3e4..7ee5cdc 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -77,6 +77,7 @@
 076 metadata rw udf auto quick stress
 077 acl attr auto enospc
 079 acl attr ioctl metadata auto quick
+080 auto quick
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
1.9.3


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] xfstests: generic/080 test that mmap-write updates c/mtime

[Dave Chinner]

On Wed, Mar 04, 2015 at 06:37:24PM +0200, Boaz Harrosh wrote:
> From: Yigal Korman <yigal@plexistor.com>
> 
> when using mmap() for file i/o, writing to the file should update
> it's c/mtime. Specifically if we first mmap-read from a page, then
> memap-write to the same page.
> 
> This test was failing for the initial submission of DAX because
> pfn based mapping do not have an page_mkwrite called for them.
> The new Kernel patches that introduce pfn_mkwrite fixes this test.

This is a lot more complex than it needs to be - xfs_io does
everything we already need, so the test really just needs to
follow the template set out by generic/309. i.e:


# pattern the file.
$XFS_IO_PROG -f -c "pwrite 0 64k" -c fsync $testfile | _filter_xfs_io

# sample timestamps.
mtime1=`stat -c %Y $testfile`
ctime1=`stat -c %Z $testfile`

# map read followed by map write to trigger timestamp change
sleep 2
$XFS_IO_PROG -c "mmap 0 64k" -c "mread 0 64k" -c "mwrite 0 4k" $testfile |_filter_xfs_io

# sample and check timestamps have changed.
mtime2=`stat -c %Y $testsfile`
ctime2=`stat -c %Z $testsfile`

if [ "$mtime1" == "$mtime2" ]; then
        echo "mtime not updated"
        let status=$status+1
fi
if [ "$ctime1" == "$ctime2" ]; then
        echo "ctime not updated"
        let status=$status+1
fi

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/3 v2] xfstest: generic/080 test that mmap-write updates c/mtime

[Boaz Harrosh]

From: Dave Chinner <dchinner@redhat.com>

when using mmap() for file i/o, writing to the file should update
it's c/mtime. Specifically if we first mmap-read from a page, then
memap-write to the same page.

This test was failing for the initial submission of DAX because
pfn based mapping do not have an page_mkwrite called for them.
The new Kernel patches that introduce pfn_mkwrite fixes this test.

Written by Dave Chinner but edited and tested by:
	Omer Zilberberg

Tested-by: Omer Zilberberg <omzg@plexistor.com>
Signed-off-by: Omer Zilberberg <omzg@plexistor.com>
Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
Dave hands-up man, it looks like you edited this directly
in the email, but there was not even a single typo.

We have tested this both with and without the pfn_mkwrite patch.
And it works as expected fails without and success with.

Thanks

 tests/generic/080     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/080.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 82 insertions(+)
 create mode 100755 tests/generic/080
 create mode 100644 tests/generic/080.out

diff --git a/tests/generic/080 b/tests/generic/080
new file mode 100755
index 0000000..2bc580d
--- /dev/null
+++ b/tests/generic/080
@@ -0,0 +1,79 @@
+#! /bin/bash
+# FS QA Test No. 080
+#
+# Verify that mtime is updated when writing to mmap-ed pages
+#
+#-----------------------------------------------------------------------
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=0
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+    rm -f $TEST_DIR/mmap_mtime_testfile
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os IRIX Linux
+_require_test
+
+echo "Silence is golden."
+rm -f $seqres.full
+
+# pattern the file.
+testfile=$TEST_DIR/mmap_mtime_testfile
+$XFS_IO_PROG -f -c "pwrite 0 4k" -c fsync $testfile >> $seqres.full
+
+# sample timestamps.
+mtime1=`stat -c %Y $testfile`
+ctime1=`stat -c %Z $testfile`
+echo "before mwrite: $mtime1 $ctime1" >> $seqres.full
+
+# map read followed by map write to trigger timestamp change
+sleep 2
+$XFS_IO_PROG -c "mmap 0 4k" -c "mread 0 4k" -c "mwrite 0 4k" $testfile |_filter_xfs_io >> $seqres.full
+
+# sample and verify that timestamps have changed.
+mtime2=`stat -c %Y $testfile`
+ctime2=`stat -c %Z $testfile`
+echo "after mwrite : $mtime2 $ctime2" >> $seqres.full
+
+if [ "$mtime1" == "$mtime2" ]; then
+	echo "mtime not updated"
+	let status=$status+1
+fi
+if [ "$ctime1" == "$ctime2" ]; then
+	echo "ctime not updated"
+	let status=$status+1
+fi
+
+exit
diff --git a/tests/generic/080.out b/tests/generic/080.out
new file mode 100644
index 0000000..cccac52
--- /dev/null
+++ b/tests/generic/080.out
@@ -0,0 +1,2 @@
+QA output created by 080
+Silence is golden.
diff --git a/tests/generic/group b/tests/generic/group
index 11ce3e4..7ee5cdc 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -77,6 +77,7 @@
 076 metadata rw udf auto quick stress
 077 acl attr auto enospc
 079 acl attr ioctl metadata auto quick
+080 auto quick
 083 rw auto enospc stress
 088 perms auto quick
 089 metadata auto
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3 v2] xfstest: generic/080 test that mmap-write updates c/mtime

[Boaz Harrosh]

I again forgot to CC: fstests@vger.kernel.org

Thanks
Boaz

On 03/05/2015 04:02 PM, Boaz Harrosh wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> when using mmap() for file i/o, writing to the file should update
> it's c/mtime. Specifically if we first mmap-read from a page, then
> memap-write to the same page.
> 
> This test was failing for the initial submission of DAX because
> pfn based mapping do not have an page_mkwrite called for them.
> The new Kernel patches that introduce pfn_mkwrite fixes this test.
> 
> Written by Dave Chinner but edited and tested by:
> 	Omer Zilberberg
> 
> Tested-by: Omer Zilberberg <omzg@plexistor.com>
> Signed-off-by: Omer Zilberberg <omzg@plexistor.com>
> Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
> ---
> Dave hands-up man, it looks like you edited this directly
> in the email, but there was not even a single typo.
> 
> We have tested this both with and without the pfn_mkwrite patch.
> And it works as expected fails without and success with.
> 
> Thanks
> 
>  tests/generic/080     | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/080.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 82 insertions(+)
>  create mode 100755 tests/generic/080
>  create mode 100644 tests/generic/080.out
> 
> diff --git a/tests/generic/080 b/tests/generic/080
> new file mode 100755
> index 0000000..2bc580d
> --- /dev/null
> +++ b/tests/generic/080
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# FS QA Test No. 080
> +#
> +# Verify that mtime is updated when writing to mmap-ed pages
> +#
> +#-----------------------------------------------------------------------
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=0
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -f $tmp.*
> +    rm -f $TEST_DIR/mmap_mtime_testfile
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os IRIX Linux
> +_require_test
> +
> +echo "Silence is golden."
> +rm -f $seqres.full
> +
> +# pattern the file.
> +testfile=$TEST_DIR/mmap_mtime_testfile
> +$XFS_IO_PROG -f -c "pwrite 0 4k" -c fsync $testfile >> $seqres.full
> +
> +# sample timestamps.
> +mtime1=`stat -c %Y $testfile`
> +ctime1=`stat -c %Z $testfile`
> +echo "before mwrite: $mtime1 $ctime1" >> $seqres.full
> +
> +# map read followed by map write to trigger timestamp change
> +sleep 2
> +$XFS_IO_PROG -c "mmap 0 4k" -c "mread 0 4k" -c "mwrite 0 4k" $testfile |_filter_xfs_io >> $seqres.full
> +
> +# sample and verify that timestamps have changed.
> +mtime2=`stat -c %Y $testfile`
> +ctime2=`stat -c %Z $testfile`
> +echo "after mwrite : $mtime2 $ctime2" >> $seqres.full
> +
> +if [ "$mtime1" == "$mtime2" ]; then
> +	echo "mtime not updated"
> +	let status=$status+1
> +fi
> +if [ "$ctime1" == "$ctime2" ]; then
> +	echo "ctime not updated"
> +	let status=$status+1
> +fi
> +
> +exit
> diff --git a/tests/generic/080.out b/tests/generic/080.out
> new file mode 100644
> index 0000000..cccac52
> --- /dev/null
> +++ b/tests/generic/080.out
> @@ -0,0 +1,2 @@
> +QA output created by 080
> +Silence is golden.
> diff --git a/tests/generic/group b/tests/generic/group
> index 11ce3e4..7ee5cdc 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -77,6 +77,7 @@
>  076 metadata rw udf auto quick stress
>  077 acl attr auto enospc
>  079 acl attr ioctl metadata auto quick
> +080 auto quick
>  083 rw auto enospc stress
>  088 perms auto quick
>  089 metadata auto
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/3] mm: New pfn_mkwrite same as page_mkwrite for VM_PFNMAP

[Boaz Harrosh]

From: Yigal Korman <yigal@plexistor.com>

This will allow FS that uses VM_PFNMAP | VM_MIXEDMAP (no page structs)
to get notified when access is a write to a read-only PFN.

This can happen if we mmap() a file then first mmap-read from it
to page-in a read-only PFN, than we mmap-write to the same page.

We need this functionality to fix a DAX bug, where in the scenario
above we fail to set ctime/mtime though we modified the file.
An xfstest is attached to this patchset that shows the failure
and the fix. (A DAX patch will follow)

This functionality is extra important for us, because upon
dirtying of a pmem page we also want to RDMA the page to a
remote cluster node.

We define a new pfn_mkwrite and do not reuse page_mkwrite because
  1 - The name ;-)
  2 - But mainly because it would take a very long and tedious
      audit of all page_mkwrite functions of VM_MIXEDMAP/VM_PFNMAP
      users. To make sure they do not now CRASH. For example current
      DAX code (which this is for) would crash.
      If we would want to reuse page_mkwrite, We will need to first
      patch all users, so to not-crash-on-no-page. Then enable this
      patch. But even if I did that I would not sleep so well at night.
      Adding a new vector is the safest thing to do, and is not that
      expensive. an extra pointer at a static function vector per driver.
      Also the new vector is better for performance, because else we
      Will call all current Kernel vectors, so to:
	check-ha-no-page-do-nothing and return.

No need to call it from do_shared_fault because do_wp_page is called to
change pte permissions anyway.

CC: Matthew Wilcox <matthew.r.wilcox@intel.com>
CC: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
CC: Jan Kara <jack@suse.cz>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Hugh Dickins <hughd@google.com>
CC: Mel Gorman <mgorman@suse.de>
CC: linux-mm@kvack.org

Signed-off-by: Yigal Korman <yigal@plexistor.com>
Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 include/linux/mm.h |  2 ++
 mm/memory.c        | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 47a9392..1cd820c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -250,6 +250,8 @@ struct vm_operations_struct {
 	/* notification that a previously read-only page is about to become
 	 * writable, if an error is returned it will cause a SIGBUS */
 	int (*page_mkwrite)(struct vm_area_struct *vma, struct vm_fault *vmf);
+	/* same as page_mkwrite when using VM_PFNMAP|VM_MIXEDMAP */
+	int (*pfn_mkwrite)(struct vm_area_struct *vma, struct vm_fault *vmf);
 
 	/* called by access_process_vm when get_user_pages() fails, typically
 	 * for use by special VMAs that can switch between memory and hardware
diff --git a/mm/memory.c b/mm/memory.c
index 8068893..ebef8f6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1982,6 +1982,22 @@ static int do_page_mkwrite(struct vm_area_struct *vma, struct page *page,
 	return ret;
 }
 
+static int do_pfn_mkwrite(struct vm_area_struct *vma, unsigned long address)
+{
+	struct vm_fault vmf;
+
+	if (!vma->vm_ops || !vma->vm_ops->pfn_mkwrite)
+		return 0;
+
+	vmf.page = 0;
+	vmf.pgoff = (((address & PAGE_MASK) - vma->vm_start) >> PAGE_SHIFT) +
+			vma->vm_pgoff;
+	vmf.virtual_address = (void __user *)(address & PAGE_MASK);
+	vmf.flags = FAULT_FLAG_WRITE|FAULT_FLAG_MKWRITE;
+
+	return vma->vm_ops->pfn_mkwrite(vma, &vmf);
+}
+
 /*
  * This routine handles present pages, when users try to write
  * to a shared page. It is done by copying the page to a new address
@@ -2025,8 +2041,17 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		 * accounting on raw pfn maps.
 		 */
 		if ((vma->vm_flags & (VM_WRITE|VM_SHARED)) ==
-				     (VM_WRITE|VM_SHARED))
+				     (VM_WRITE|VM_SHARED)) {
+			pte_unmap_unlock(page_table, ptl);
+			ret = do_pfn_mkwrite(vma, address);
+			if (ret & VM_FAULT_ERROR)
+				return ret;
+			page_table = pte_offset_map_lock(mm, pmd, address,
+							 &ptl);
+			if (!pte_same(*page_table, orig_pte))
+				goto unlock;
 			goto reuse;
+		}
 		goto gotten;
 	}
 
-- 
1.9.3


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3] DAX: use pfn_mkwrite to update c/mtime

[Boaz Harrosh]

From: Yigal Korman <yigal@plexistor.com>

Without this patch, c/mtime is not updated correctly when mmap'ed page is
first read from and then written to.

A new xfstest is submitted for testing this (generic/080)

Signed-off-by: Yigal Korman <yigal@plexistor.com>
Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 fs/dax.c           | 13 +++++++++++++
 fs/ext2/file.c     |  1 +
 fs/ext4/file.c     |  1 +
 include/linux/fs.h |  1 +
 4 files changed, 16 insertions(+)

diff --git a/fs/dax.c b/fs/dax.c
index ed1619e..cd63adc 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -464,6 +464,19 @@ int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
 EXPORT_SYMBOL_GPL(dax_fault);
 
 /**
+ * dax_pfn_mkwrite - handle first write to DAX page
+ * @vma: The virtual memory area where the fault occurred
+ * @vmf: The description of the fault
+ *
+ */
+int dax_pfn_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	file_update_time(vma->vm_file);
+	return VM_FAULT_NOPAGE;
+}
+EXPORT_SYMBOL_GPL(dax_pfn_mkwrite);
+
+/**
  * dax_zero_page_range - zero a range within a page of a DAX file
  * @inode: The file being truncated
  * @from: The file offset that is being truncated to
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index e317017..866a3ce 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -39,6 +39,7 @@ static int ext2_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 static const struct vm_operations_struct ext2_dax_vm_ops = {
 	.fault		= ext2_dax_fault,
 	.page_mkwrite	= ext2_dax_mkwrite,
+	.pfn_mkwrite	= dax_pfn_mkwrite,
 };
 
 static int ext2_file_mmap(struct file *file, struct vm_area_struct *vma)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 33a09da..b43a7a6 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -206,6 +206,7 @@ static int ext4_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 static const struct vm_operations_struct ext4_dax_vm_ops = {
 	.fault		= ext4_dax_fault,
 	.page_mkwrite	= ext4_dax_mkwrite,
+	.pfn_mkwrite	= dax_pfn_mkwrite,
 };
 #else
 #define ext4_dax_vm_ops	ext4_file_vm_ops
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4d71b5..24af817 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2597,6 +2597,7 @@ int dax_clear_blocks(struct inode *, sector_t block, long size);
 int dax_zero_page_range(struct inode *, loff_t from, unsigned len, get_block_t);
 int dax_truncate_page(struct inode *, loff_t from, get_block_t);
 int dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t);
+int dax_pfn_mkwrite(struct vm_area_struct *, struct vm_fault *);
 #define dax_mkwrite(vma, vmf, gb)	dax_fault(vma, vmf, gb)
 
 #ifdef CONFIG_BLOCK
-- 
1.9.3


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3] DAX: use pfn_mkwrite to update c/mtime

[Jan Kara]

On Wed 04-03-15 18:48:06, Boaz Harrosh wrote:
> From: Yigal Korman <yigal@plexistor.com>
> 
> Without this patch, c/mtime is not updated correctly when mmap'ed page is
> first read from and then written to.
> 
> A new xfstest is submitted for testing this (generic/080)
> 
> Signed-off-by: Yigal Korman <yigal@plexistor.com>
> Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
> ---
>  fs/dax.c           | 13 +++++++++++++
>  fs/ext2/file.c     |  1 +
>  fs/ext4/file.c     |  1 +
>  include/linux/fs.h |  1 +
>  4 files changed, 16 insertions(+)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index ed1619e..cd63adc 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -464,6 +464,19 @@ int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
>  EXPORT_SYMBOL_GPL(dax_fault);
>  
>  /**
> + * dax_pfn_mkwrite - handle first write to DAX page
> + * @vma: The virtual memory area where the fault occurred
> + * @vmf: The description of the fault
> + *
> + */
> +int dax_pfn_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> +	file_update_time(vma->vm_file);
> +	return VM_FAULT_NOPAGE;
> +}
> +EXPORT_SYMBOL_GPL(dax_pfn_mkwrite);
  Hum, you likely want a freeze protection in there as well
(sb_start_pagefault() and sb_end_pagefault()). Don't you?  Otherwise user
could start writing to the page while filesystem is frozen.  That's another
bug you could mention in your description.

								Honza

> +
> +/**
>   * dax_zero_page_range - zero a range within a page of a DAX file
>   * @inode: The file being truncated
>   * @from: The file offset that is being truncated to
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index e317017..866a3ce 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -39,6 +39,7 @@ static int ext2_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
>  static const struct vm_operations_struct ext2_dax_vm_ops = {
>  	.fault		= ext2_dax_fault,
>  	.page_mkwrite	= ext2_dax_mkwrite,
> +	.pfn_mkwrite	= dax_pfn_mkwrite,
>  };
>  
>  static int ext2_file_mmap(struct file *file, struct vm_area_struct *vma)
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 33a09da..b43a7a6 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -206,6 +206,7 @@ static int ext4_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
>  static const struct vm_operations_struct ext4_dax_vm_ops = {
>  	.fault		= ext4_dax_fault,
>  	.page_mkwrite	= ext4_dax_mkwrite,
> +	.pfn_mkwrite	= dax_pfn_mkwrite,
>  };
>  #else
>  #define ext4_dax_vm_ops	ext4_file_vm_ops
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b4d71b5..24af817 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2597,6 +2597,7 @@ int dax_clear_blocks(struct inode *, sector_t block, long size);
>  int dax_zero_page_range(struct inode *, loff_t from, unsigned len, get_block_t);
>  int dax_truncate_page(struct inode *, loff_t from, get_block_t);
>  int dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t);
> +int dax_pfn_mkwrite(struct vm_area_struct *, struct vm_fault *);
>  #define dax_mkwrite(vma, vmf, gb)	dax_fault(vma, vmf, gb)
>  
>  #ifdef CONFIG_BLOCK
> -- 
> 1.9.3
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/3 v2] dax: use pfn_mkwrite to update c/mtime + freeze protection

[Boaz Harrosh]

From: Yigal Korman <yigal@plexistor.com>

[v1]
Without this patch, c/mtime is not updated correctly when mmap'ed page is
first read from and then written to.

A new xfstest is submitted for testing this (generic/080)

[v2]
Jan Kara has pointed out that if we add the
sb_start/end_pagefault pair in the new pfn_mkwrite we
are then fixing another bug where: A user could start
writing to the page while filesystem is frozen.

CC: Jan Kara <jack@suse.cz>
Signed-off-by: Yigal Korman <yigal@plexistor.com>
Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 fs/dax.c           | 17 +++++++++++++++++
 fs/ext2/file.c     |  1 +
 fs/ext4/file.c     |  1 +
 include/linux/fs.h |  1 +
 4 files changed, 20 insertions(+)

diff --git a/fs/dax.c b/fs/dax.c
index ed1619e..d0bd1f4 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -464,6 +464,23 @@ int dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
 EXPORT_SYMBOL_GPL(dax_fault);
 
 /**
+ * dax_pfn_mkwrite - handle first write to DAX page
+ * @vma: The virtual memory area where the fault occurred
+ * @vmf: The description of the fault
+ *
+ */
+int dax_pfn_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	struct super_block *sb = file_inode(vma->vm_file)->i_sb;
+
+	sb_start_pagefault(sb);
+	file_update_time(vma->vm_file);
+	sb_end_pagefault(sb);
+	return VM_FAULT_NOPAGE;
+}
+EXPORT_SYMBOL_GPL(dax_pfn_mkwrite);
+
+/**
  * dax_zero_page_range - zero a range within a page of a DAX file
  * @inode: The file being truncated
  * @from: The file offset that is being truncated to
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index e317017..866a3ce 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -39,6 +39,7 @@ static int ext2_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 static const struct vm_operations_struct ext2_dax_vm_ops = {
 	.fault		= ext2_dax_fault,
 	.page_mkwrite	= ext2_dax_mkwrite,
+	.pfn_mkwrite	= dax_pfn_mkwrite,
 };
 
 static int ext2_file_mmap(struct file *file, struct vm_area_struct *vma)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 33a09da..b43a7a6 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -206,6 +206,7 @@ static int ext4_dax_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 static const struct vm_operations_struct ext4_dax_vm_ops = {
 	.fault		= ext4_dax_fault,
 	.page_mkwrite	= ext4_dax_mkwrite,
+	.pfn_mkwrite	= dax_pfn_mkwrite,
 };
 #else
 #define ext4_dax_vm_ops	ext4_file_vm_ops
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4d71b5..24af817 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2597,6 +2597,7 @@ int dax_clear_blocks(struct inode *, sector_t block, long size);
 int dax_zero_page_range(struct inode *, loff_t from, unsigned len, get_block_t);
 int dax_truncate_page(struct inode *, loff_t from, get_block_t);
 int dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t);
+int dax_pfn_mkwrite(struct vm_area_struct *, struct vm_fault *);
 #define dax_mkwrite(vma, vmf, gb)	dax_fault(vma, vmf, gb)
 
 #ifdef CONFIG_BLOCK
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3 v2] dax: use pfn_mkwrite to update c/mtime + freeze protection

[Boaz Harrosh]

On 03/05/2015 11:24 AM, Boaz Harrosh wrote:
> 
> [v1]
> Without this patch, c/mtime is not updated correctly when mmap'ed page is
> first read from and then written to.
> 
> A new xfstest is submitted for testing this (generic/080)
> 
> [v2]
> Jan Kara has pointed out that if we add the
> sb_start/end_pagefault pair in the new pfn_mkwrite we
> are then fixing another bug where: A user could start
> writing to the page while filesystem is frozen.
> 

Thanks Jan.

Just as curiosity, does the freezing code goes and turns all mappings
into read-only, Also for pfn mapping?

Do you think there is already an xfstest freezing test that should now
fail, and will succeed after this patch (v2). Something like:
  * mmap-read/write before the freeze
  * freeze the fs
  * Another thread tries to mmap-write, should get stuck
  * unfreeze the fs
  * Now mmap-writer continues

Thanks again
Boaz

> CC: Jan Kara <jack@suse.cz>
> Signed-off-by: Yigal Korman <yigal@plexistor.com>
> Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
<>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3 v2] dax: use pfn_mkwrite to update c/mtime + freeze protection

[Jan Kara]

On Thu 05-03-15 11:32:25, Boaz Harrosh wrote:
> On 03/05/2015 11:24 AM, Boaz Harrosh wrote:
> > 
> > [v1]
> > Without this patch, c/mtime is not updated correctly when mmap'ed page is
> > first read from and then written to.
> > 
> > A new xfstest is submitted for testing this (generic/080)
> > 
> > [v2]
> > Jan Kara has pointed out that if we add the
> > sb_start/end_pagefault pair in the new pfn_mkwrite we
> > are then fixing another bug where: A user could start
> > writing to the page while filesystem is frozen.
> > 
> 
> Thanks Jan.
> 
> Just as curiosity, does the freezing code goes and turns all mappings
> into read-only, Also for pfn mapping?
  Hum, that's a good question. Probably we don't end up doing that. For
normal filesystems we sync all inodes which also writeprotects all pages
(in clear_page_dirty_for_io() - for normal filesystems we know that if page
is writeably mapped it is dirty). However this won't happen for pfn
mapping as we don't have dirty pages. So we probably need dax_freeze()
implementation that will walk through all inodes with writeable mappings and
writeprotect them.

> Do you think there is already an xfstest freezing test that should now
> fail, and will succeed after this patch (v2). Something like:
>   * mmap-read/write before the freeze
>   * freeze the fs
>   * Another thread tries to mmap-write, should get stuck
>   * unfreeze the fs
>   * Now mmap-writer continues
  I don't remember there would be any test to specifically test this.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3 v2] dax: use pfn_mkwrite to update c/mtime + freeze protection

[Boaz Harrosh]

On 03/05/2015 12:35 PM, Jan Kara wrote:
> On Thu 05-03-15 11:32:25, Boaz Harrosh wrote:
>> On 03/05/2015 11:24 AM, Boaz Harrosh wrote:
<>
>>
>> Just as curiosity, does the freezing code goes and turns all mappings
>> into read-only, Also for pfn mapping?
>   Hum, that's a good question. Probably we don't end up doing that. For
>
> normal filesystems we sync all inodes which also writeprotects all pages
> (in clear_page_dirty_for_io() - for normal filesystems we know that if page
> is writeably mapped it is dirty). However this won't happen for pfn
> mapping as we don't have dirty pages. So we probably need dax_freeze()
> implementation that will walk through all inodes with writeable mappings and
> writeprotect them.
> 

I'll go head and try my shot on implementing a dax_freeze(). But I will
please need help with where to call it from.

Probably something like:
	if (IS_DAX(inode))
		dax_freeze(inode);
	else
		sync(inode)

So to share the for-all-inodes-in-sb loop. And also the IS_DAX(inode)
is per inode, for example dirs need the regular sync (if they are using page cache)

>> Do you think there is already an xfstest freezing test that should now
>> fail, and will succeed after this patch (v2). Something like:
>>   * mmap-read/write before the freeze
>>   * freeze the fs
>>   * Another thread tries to mmap-write, should get stuck
>>   * unfreeze the fs
>>   * Now mmap-writer continues
>   I don't remember there would be any test to specifically test this.
> 

OK Thanks, I was hopping we should already test for mmap vs freeze. This
is not special for our case. Actually mmap is the most fragile access.

> 								Honza
> 

Thanks
Boaz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/3 v2] dax: use pfn_mkwrite to update c/mtime + freeze protection

[Jan Kara]

On Thu 05-03-15 12:47:30, Boaz Harrosh wrote:
> On 03/05/2015 12:35 PM, Jan Kara wrote:
> > On Thu 05-03-15 11:32:25, Boaz Harrosh wrote:
> >> On 03/05/2015 11:24 AM, Boaz Harrosh wrote:
> <>
> >>
> >> Just as curiosity, does the freezing code goes and turns all mappings
> >> into read-only, Also for pfn mapping?
> >   Hum, that's a good question. Probably we don't end up doing that. For
> >
> > normal filesystems we sync all inodes which also writeprotects all pages
> > (in clear_page_dirty_for_io() - for normal filesystems we know that if page
> > is writeably mapped it is dirty). However this won't happen for pfn
> > mapping as we don't have dirty pages. So we probably need dax_freeze()
> > implementation that will walk through all inodes with writeable mappings and
> > writeprotect them.
> > 
> 
> I'll go head and try my shot on implementing a dax_freeze(). But I will
> please need help with where to call it from.
> 
> Probably something like:
> 	if (IS_DAX(inode))
> 		dax_freeze(inode);
> 	else
> 		sync(inode)
  We normally call sync_filesystem() from fs/superblock:freeze_super(). For
DAX filesystems we'd also need to call the special function after that.
Maybe dax_freeze() isn't the best name. It could be called something like
dax_writeprotect(sb) or something like that.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>