Skip to contentSkip to conversation

msync: fix incorrect fstart calculation

PATCH In review Namjae Jeon 6 participants 7 messages Jun 27, 2014 – Jun 28, 2014

Subsystems (2)
Memory management Memory mapping
lore
Share & tools
lore

Namjae Jeon

author
#0 lore raw

PATCH 1/1 msync: fix incorrect fstart calculation

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Lukáš Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
---
 mm/msync.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/msync.c +2 -1
index a5c6736..ad97dce 100644
@@ -78,7 +78,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 			goto out_unlock;
 		}
 		file = vma->vm_file;
-		fstart = start + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+		fstart = (start - vma->vm_start) +
+			 ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
 		fend = fstart + (min(end, vma->vm_end) - start) - 1;
 		start = vma->vm_end;
 		if ((flags & MS_SYNC) && file &&
Signature · 7 lines
1.7.11-rc0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Reviewed-by Tested-by on v1 1/1
#1 lore raw
Quoted 10 lines · Namjae Jeon

Date: Fri, 27 Jun 2014 20:38:49 +0900
From: Namjae Jeon <[email protected]>
To: 'Andrew Morton' <[email protected]>
Cc: [email protected], linux-ext4 <[email protected]>,
Lukáš Czerner <[email protected]>,
'Matthew Wilcox' <[email protected]>, 'Eric Whitney' <[email protected]>,
Ashish Sangwan <[email protected]>
Subject: [PATCH] msync: fix incorrect fstart calculation

Quoted 4 lines

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Looks good to me and it fixes the issues with data=journal on ext4.

Reviewed-by Lukas Czerner <[email protected]>
Tested-by Lukas Czerner <[email protected]>
Quoted 25 lines
Cc Matthew Wilcox <[email protected]>
Cc Lukáš Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
---
 mm/msync.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/msync.c +2 -1
index a5c6736..ad97dce 100644
@@ -78,7 +78,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 			goto out_unlock;
 		}
 		file = vma->vm_file;
-		fstart = start + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+		fstart = (start - vma->vm_start) +
+			 ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
 		fend = fstart + (min(end, vma->vm_end) - start) - 1;
 		start = vma->vm_end;
 		if ((flags & MS_SYNC) && file &&
Acked-by on v1 1/1
#2 lore raw
In reply to Namjae Jeon
Acked-by Matthew Wilcox <[email protected]>

________________________________________
From: Namjae Jeon [[email protected]]
Sent: June 27, 2014 4:38 AM
To: 'Andrew Morton'
Cc: [email protected]; linux-ext4; Lukáš Czerner; Wilcox, Matthew R; 'Eric Whitney'; Ashish Sangwan
Subject: [PATCH] msync: fix incorrect fstart calculation

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Lukáš Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
---
 mm/msync.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/msync.c +2 -1
index a5c6736..ad97dce 100644
@@ -78,7 +78,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
                        goto out_unlock;
                }
                file = vma->vm_file;
-               fstart = start + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+               fstart = (start - vma->vm_start) +
+                        ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
                fend = fstart + (min(end, vma->vm_end) - start) - 1;
                start = vma->vm_end;
                if ((flags & MS_SYNC) && file &&

--
1.7.11-rc0

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

#3 lore raw

I can confirm that this patch corrects the ext4 regressions I reported on 3.16-rc1 for data_journal.

Additionally, it corrects regressions for two other tests I have not yet reported. Those tests include generic/263 when running with the data=journal mount option, and generic/219 (a quota test that doesn't use fsx) when running with all xfstests-bld scenarios (4k, ext4, nojournal, 1k, etc.) with the exception of bigalloc. The generic/219 failure on bigalloc in 3.16-rc1 is not a regression, and was present in earlier releases.

With this patch, ext4 3.16-rc3 regression results on x64_64 should look much more like 3.15 final.

Thanks guys!
Eric

* Wilcox, Matthew R <[email protected]>:

Quoted 37 lines
Acked-by Matthew Wilcox <[email protected]>

________________________________________
From: Namjae Jeon [[email protected]]
Sent: June 27, 2014 4:38 AM
To: 'Andrew Morton'
Cc: [email protected]; linux-ext4; Lukáš Czerner; Wilcox, Matthew R; 'Eric Whitney'; Ashish Sangwan
Subject: [PATCH] msync: fix incorrect fstart calculation

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Lukáš Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
---
 mm/msync.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
mm/msync.c +2 -2
index a5c6736..ad97dce 100644
@@ -78,7 +78,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
                        goto out_unlock;
                }
                file = vma->vm_file;
-               fstart = start + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+               fstart = (start - vma->vm_start) +
+                        ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
                fend = fstart + (min(end, vma->vm_end) - start) - 1;
                start = vma->vm_end;
                if ((flags & MS_SYNC) && file &&
--
Quoted 2 lines

1.7.11-rc0

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

Reviewed-by on v1 1/1
#4 lore raw
In reply to Namjae Jeon
Quoted 10 lines · Namjae Jeon

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Lukáš Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
Reviewed-by Theodore Ts'o <[email protected]>

Andrew, do you want to take this in the mm tree, or shall I take it in the ext4 tree? I would prefer if we could get this pushed to Linus as soon as possible, since it fixes a regression.

- Ted

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

maintainer
#5 lore raw
Quoted 12 lines · On Sat, 28 Jun 2014 15:14:25 -0400 "Theodore Ts'o" <[email protected]> wrote:
Quoted 10 lines · Namjae Jeon

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Luk____ Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
Quoted 3 lines
Reviewed-by Theodore Ts'o <[email protected]>

Andrew, do you want to take this in the mm tree,

I have done so.

Quoted 3 lines

or shall I take it in
the ext4 tree? I would prefer if we could get this pushed to Linus as soon as possible, since it fixes a regression.

Yep, I'll get it over to Linus early next week.

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

#6 lore raw
Quoted 14 lines · Andrew Morton
Quoted 12 lines · On Sat, 28 Jun 2014 15:14:25 -0400 "Theodore Ts'o" <[email protected]> wrote:
Quoted 10 lines · Namjae Jeon

Fix a regression caused by Commit 7fc34a62ca mm/msync.c: sync only the requested range in msync().
xfstests generic/075 fail occured on ext4 data=journal mode because the intended range was not syncing due to wrong fstart calculation.

Cc Matthew Wilcox <[email protected]>
Cc Luk____ Czerner <[email protected]>
Reported-by Eric Whitney <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
Signed-off-by Ashish Sangwan <[email protected]>
Quoted 5 lines
Quoted 3 lines
Reviewed-by Theodore Ts'o <[email protected]>

Andrew, do you want to take this in the mm tree,

I have done so.

Great, thanks!!

- Ted

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