On Monday 06 Jul 2015 11:17:38 Liu Bo wrote:
> On Fri, Jul 03, 2015 at 03:38:00PM +0530, Chandan Rajendra wrote:
> > On Wednesday 01 Jul 2015 22:47:10 Liu Bo wrote:
> > > On Mon, Jun 01, 2015 at 08:52:47PM +0530, Chandan Rajendra wrote:
> > > > In subpagesize-blocksize scenario it is not sufficient to search using
> > > > the
> > > > first byte of the page to make sure that there are no ordered extents
> > > > present across the page. Fix this.
> > > > 
> > > > Signed-off-by: Chandan Rajendra <chan...@linux.vnet.ibm.com>
> > > > ---
> > > > 
> > > >  fs/btrfs/extent_io.c | 3 ++-
> > > >  fs/btrfs/inode.c     | 4 ++--
> > > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > > > index 14b4e05..0b017e1 100644
> > > > --- a/fs/btrfs/extent_io.c
> > > > +++ b/fs/btrfs/extent_io.c
> > > > @@ -3244,7 +3244,8 @@ static int __extent_read_full_page(struct
> > > > extent_io_tree *tree,>
> > > > 
> > > >         while (1) {
> > > >         
> > > >                 lock_extent(tree, start, end);
> > > > 
> > > > -               ordered = btrfs_lookup_ordered_extent(inode, start);
> > > > +               ordered = btrfs_lookup_ordered_range(inode, start,
> > > > +                                               PAGE_CACHE_SIZE);
> > > 
> > > A minor suggestion, it'd be better to include the new prototype in the
> > > same patch, which will be benefit to later cherry-picking or reverting.
> > 
> > Liu, The definition of btrfs_lookup_ordered_range() is already part of
> > the mainline kernel.
> 
> Ah, I didn't recognize the difference of btrfs_lookup_ordered_extent and
> btrfs_lookup_ordered_range, sorry.
> 
> > > >                 if (!ordered)
> > > >                 
> > > >                         break;
> > > >                 
> > > >                 unlock_extent(tree, start, end);
> > > > 
> > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > > > index e9bab73..8b4aaed 100644
> > > > --- a/fs/btrfs/inode.c
> > > > +++ b/fs/btrfs/inode.c
> > > > 
> > > > @@ -1976,7 +1976,7 @@ again:
> > > >         if (PagePrivate2(page))
> > > >         
> > > >                 goto out;
> > > > 
> > > > -       ordered = btrfs_lookup_ordered_extent(inode, page_start);
> > > > +       ordered = btrfs_lookup_ordered_range(inode, page_start,
> > > > PAGE_CACHE_SIZE);
> > > > 
> > > >         if (ordered) {
> > > >         
> > > >                 unlock_extent_cached(&BTRFS_I(inode)->io_tree, 
page_start,
> > > >                 
> > > >                                      page_end, &cached_state, 
GFP_NOFS);
> > > > 
> > > > @@ -8513,7 +8513,7 @@ static void btrfs_invalidatepage(struct page
> > > > *page,
> > > > unsigned int offset,>
> > > > 
> > > >         if (!inode_evicting)
> > > >         
> > > >                 lock_extent_bits(tree, page_start, page_end, 0,
> > 
> > &cached_state);
> > 
> > > > -       ordered = btrfs_lookup_ordered_extent(inode, page_start);
> > > > +       ordered = btrfs_lookup_ordered_range(inode, page_start,
> > > > PAGE_CACHE_SIZE);
> 
> It's possible for a page to hold two (or more) ordered extents here, a
> while loop is necessary to ensure that every ordered extent is processed
> properly.
>
Liu, Sorry, I had introduced the loop in the patch
"[RFC PATCH V11 14/21] Btrfs: subpagesize-blocksize: Explicitly Track I/O
status of blocks of an ordered extent". I will pull the loop to this patch for
the next version of that patchset. 

> Thanks,
> 
> -liubo
> 
> > > >         if (ordered) {
> > > >         
> > > >                 /*
> > > >                 
> > > >                  * IO on this page will never be started, so we need

-- 
chandan

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to