[PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[Liu Bo]

Commit "d0b7da88 Btrfs: btrfs_page_mkwrite: Reserve space in sectorsized units"
did this, but btrfs_lookup_ordered_range expects a 'length' rather than a
'page_end'.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
Is this a candidate for stable?

 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4e02426..366cf0b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9023,7 +9023,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * we can't set the delalloc bits if there are pending ordered
 	 * extents.  Drop our locks and wait for them to finish
 	 */
-	ordered = btrfs_lookup_ordered_range(inode, page_start, page_end);
+	ordered = btrfs_lookup_ordered_range(inode, page_start, PAGE_SIZE);
 	if (ordered) {
 		unlock_extent_cached(io_tree, page_start, page_end,
 				     &cached_state, GFP_NOFS);
-- 
2.5.5

Re: [PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[Chandan Rajendra]

On Tuesday, January 24, 2017 03:58:51 PM Liu Bo wrote:
> Commit "d0b7da88 Btrfs: btrfs_page_mkwrite: Reserve space in sectorsized units"
> did this, but btrfs_lookup_ordered_range expects a 'length' rather than a
> 'page_end'.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
> Is this a candidate for stable?
> 
>  fs/btrfs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4e02426..366cf0b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9023,7 +9023,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	 * we can't set the delalloc bits if there are pending ordered
>  	 * extents.  Drop our locks and wait for them to finish
>  	 */
> -	ordered = btrfs_lookup_ordered_range(inode, page_start, page_end);
> +	ordered = btrfs_lookup_ordered_range(inode, page_start, PAGE_SIZE);
>  	if (ordered) {
>  		unlock_extent_cached(io_tree, page_start, page_end,
>  				     &cached_state, GFP_NOFS);
> 

Thanks for fixing this,
Reviewed-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>

As for the question about whether this commit should be merged into the stable
trees ... I am not sure about that since I don't notice any sort of filesystem
corruption that can be caused by the current code i.e. With the existing code,
apart from any ordered extents that map the page in question, we are most
likely to be *unnecessarily* starting i/o on ordered extents that don't map
the file offset range covered by the page. Chris, Josef or David, Please let
us know your thoughts on this.

-- 
chandan

Re: [PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[Liu Bo]

On Wed, Jan 25, 2017 at 01:49:09PM +0530, Chandan Rajendra wrote:
> On Tuesday, January 24, 2017 03:58:51 PM Liu Bo wrote:
> > Commit "d0b7da88 Btrfs: btrfs_page_mkwrite: Reserve space in sectorsized units"
> > did this, but btrfs_lookup_ordered_range expects a 'length' rather than a
> > 'page_end'.
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> > Is this a candidate for stable?
> > 
> >  fs/btrfs/inode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 4e02426..366cf0b 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -9023,7 +9023,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> >  	 * we can't set the delalloc bits if there are pending ordered
> >  	 * extents.  Drop our locks and wait for them to finish
> >  	 */
> > -	ordered = btrfs_lookup_ordered_range(inode, page_start, page_end);
> > +	ordered = btrfs_lookup_ordered_range(inode, page_start, PAGE_SIZE);
> >  	if (ordered) {
> >  		unlock_extent_cached(io_tree, page_start, page_end,
> >  				     &cached_state, GFP_NOFS);
> > 
> 
> Thanks for fixing this,
> Reviewed-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> 
> As for the question about whether this commit should be merged into the stable
> trees ... I am not sure about that since I don't notice any sort of filesystem
> corruption that can be caused by the current code i.e. With the existing code,
> apart from any ordered extents that map the page in question, we are most
> likely to be *unnecessarily* starting i/o on ordered extents that don't map
> the file offset range covered by the page. Chris, Josef or David, Please let
> us know your thoughts on this.

It could be a performance regression which causes fault writes have
unnecessary waits instead of a real corruption.

Thanks,

-liubo

Re: [PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[David Sterba]

On Wed, Jan 25, 2017 at 07:06:18AM -0800, Liu Bo wrote:
> On Wed, Jan 25, 2017 at 01:49:09PM +0530, Chandan Rajendra wrote:
> > On Tuesday, January 24, 2017 03:58:51 PM Liu Bo wrote:
> > > Commit "d0b7da88 Btrfs: btrfs_page_mkwrite: Reserve space in sectorsized units"
> > > did this, but btrfs_lookup_ordered_range expects a 'length' rather than a
> > > 'page_end'.
> > > 
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > ---
> > > Is this a candidate for stable?
> > > 
> > >  fs/btrfs/inode.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > > index 4e02426..366cf0b 100644
> > > --- a/fs/btrfs/inode.c
> > > +++ b/fs/btrfs/inode.c
> > > @@ -9023,7 +9023,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> > >  	 * we can't set the delalloc bits if there are pending ordered
> > >  	 * extents.  Drop our locks and wait for them to finish
> > >  	 */
> > > -	ordered = btrfs_lookup_ordered_range(inode, page_start, page_end);
> > > +	ordered = btrfs_lookup_ordered_range(inode, page_start, PAGE_SIZE);
> > >  	if (ordered) {
> > >  		unlock_extent_cached(io_tree, page_start, page_end,
> > >  				     &cached_state, GFP_NOFS);
> > > 
> > 
> > Thanks for fixing this,
> > Reviewed-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> > 
> > As for the question about whether this commit should be merged into the stable
> > trees ... I am not sure about that since I don't notice any sort of filesystem
> > corruption that can be caused by the current code i.e. With the existing code,
> > apart from any ordered extents that map the page in question, we are most
> > likely to be *unnecessarily* starting i/o on ordered extents that don't map
> > the file offset range covered by the page. Chris, Josef or David, Please let
> > us know your thoughts on this.
> 
> It could be a performance regression which causes fault writes have
> unnecessary waits instead of a real corruption.

Does not seem to be urgent for stable, but I'll consider it next time
doing a stable round updates.

Re: [PATCH] Btrfs: fix wrong argument for btrfs_lookup_ordered_range

[Caitlyn Mason]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2676 bytes --]

I don’t think I’m supposed to be on this thread – please move me to bcc! ☺ 

-- 
caitlyn mason
Facebook | University Programs
M: (508) 963-6209
E: caitmase@fb.com
 

 
 facebook.com/careers
 

On 1/26/17, 9:28 AM, "David Sterba" <dsterba@suse.cz> wrote:

    On Wed, Jan 25, 2017 at 07:06:18AM -0800, Liu Bo wrote:
    > On Wed, Jan 25, 2017 at 01:49:09PM +0530, Chandan Rajendra wrote:
    > > On Tuesday, January 24, 2017 03:58:51 PM Liu Bo wrote:
    > > > Commit "d0b7da88 Btrfs: btrfs_page_mkwrite: Reserve space in sectorsized units"
    > > > did this, but btrfs_lookup_ordered_range expects a 'length' rather than a
    > > > 'page_end'.
    > > > 
    > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
    > > > ---
    > > > Is this a candidate for stable?
    > > > 
    > > >  fs/btrfs/inode.c | 2 +-
    > > >  1 file changed, 1 insertion(+), 1 deletion(-)
    > > > 
    > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
    > > > index 4e02426..366cf0b 100644
    > > > --- a/fs/btrfs/inode.c
    > > > +++ b/fs/btrfs/inode.c
    > > > @@ -9023,7 +9023,7 @@ int btrfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
    > > >  	 * we can't set the delalloc bits if there are pending ordered
    > > >  	 * extents.  Drop our locks and wait for them to finish
    > > >  	 */
    > > > -	ordered = btrfs_lookup_ordered_range(inode, page_start, page_end);
    > > > +	ordered = btrfs_lookup_ordered_range(inode, page_start, PAGE_SIZE);
    > > >  	if (ordered) {
    > > >  		unlock_extent_cached(io_tree, page_start, page_end,
    > > >  				     &cached_state, GFP_NOFS);
    > > > 
    > > 
    > > Thanks for fixing this,
    > > Reviewed-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
    > > 
    > > As for the question about whether this commit should be merged into the stable
    > > trees ... I am not sure about that since I don't notice any sort of filesystem
    > > corruption that can be caused by the current code i.e. With the existing code,
    > > apart from any ordered extents that map the page in question, we are most
    > > likely to be *unnecessarily* starting i/o on ordered extents that don't map
    > > the file offset range covered by the page. Chris, Josef or David, Please let
    > > us know your thoughts on this.
    > 
    > It could be a performance regression which causes fault writes have
    > unnecessary waits instead of a real corruption.
    
    Does not seem to be urgent for stable, but I'll consider it next time
    doing a stable round updates.
    

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±ý»k~ÏâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm