2014-06-27 11:38:49

by Namjae Jeon

[permalink] [raw]
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(-)

diff --git a/mm/msync.c b/mm/msync.c
index a5c6736..ad97dce 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -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
ilto:"[email protected]"> [email protected] </a>


2014-06-27 11:51:11

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH] msync: fix incorrect fstart calculation

On Fri, 27 Jun 2014, Namjae Jeon wrote:

> 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
>
> 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]>

>
> 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(-)
>
> diff --git a/mm/msync.c b/mm/msync.c
> index a5c6736..ad97dce 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -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 &&
>

2014-06-27 12:43:30

by Matthew Wilcox

[permalink] [raw]
Subject: RE: [PATCH] msync: fix incorrect fstart calculation

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(-)

diff --git a/mm/msync.c b/mm/msync.c
index a5c6736..ad97dce 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -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
ilto:"[email protected]"> [email protected] </a>

2014-06-27 16:12:56

by Eric Whitney

[permalink] [raw]
Subject: Re: [PATCH] msync: fix incorrect fstart calculation

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]>:
> 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(-)
>
> diff --git a/mm/msync.c b/mm/msync.c
> index a5c6736..ad97dce 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -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>

2014-06-28 19:14:25

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] msync: fix incorrect fstart calculation

On Fri, Jun 27, 2014 at 08:38:49PM +0900, Namjae Jeon wrote:
> 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>

2014-06-28 19:44:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] msync: fix incorrect fstart calculation

On Sat, 28 Jun 2014 15:14:25 -0400 "Theodore Ts'o" <[email protected]> wrote:

> On Fri, Jun 27, 2014 at 08:38:49PM +0900, Namjae Jeon wrote:
> > 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,

I have done so.

> 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>

2014-06-28 19:57:48

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] msync: fix incorrect fstart calculation

On Sat, Jun 28, 2014 at 12:44:21PM -0700, Andrew Morton wrote:
> On Sat, 28 Jun 2014 15:14:25 -0400 "Theodore Ts'o" <[email protected]> wrote:
>
> > On Fri, Jun 27, 2014 at 08:38:49PM +0900, Namjae Jeon wrote:
> > > 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,
>
> 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>