On Monday 06 Jul 2015 18:06:33 Liu Bo wrote: > On Mon, Jun 01, 2015 at 08:52:48PM +0530, Chandan Rajendra wrote: > > In subpagesize-blocksize scenario, extent allocations for only some of the > > dirty blocks of a page can succeed, while allocation for rest of the > > blocks > > can fail. This patch allows I/O against such partially allocated ordered > > extents to be submitted. > > > > Signed-off-by: Chandan Rajendra <[email protected]> > > --- > > > > fs/btrfs/extent_io.c | 27 ++++++++++++++------------- > > fs/btrfs/inode.c | 35 ++++++++++++++++++++++------------- > > 2 files changed, 36 insertions(+), 26 deletions(-) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index 0b017e1..0110abc 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -1850,17 +1850,23 @@ int extent_clear_unlock_delalloc(struct inode > > *inode, u64 start, u64 end,> > > if (page_ops & PAGE_SET_PRIVATE2) > > > > SetPagePrivate2(pages[i]); > > > > + if (page_ops & PAGE_SET_ERROR) > > + SetPageError(pages[i]); > > + > > > > if (pages[i] == locked_page) { > > > > page_cache_release(pages[i]); > > continue; > > > > } > > > > - if (page_ops & PAGE_CLEAR_DIRTY) > > + > > + if ((page_ops & PAGE_CLEAR_DIRTY) > > + && !PagePrivate2(pages[i])) > > > > clear_page_dirty_for_io(pages[i]); > > > > - if (page_ops & PAGE_SET_WRITEBACK) > > + if ((page_ops & PAGE_SET_WRITEBACK) > > + && !PagePrivate2(pages[i])) > > > > set_page_writeback(pages[i]); > > > > - if (page_ops & PAGE_SET_ERROR) > > - SetPageError(pages[i]); > > - if (page_ops & PAGE_END_WRITEBACK) > > + > > + if ((page_ops & PAGE_END_WRITEBACK) > > + && !PagePrivate2(pages[i])) > > > > end_page_writeback(pages[i]); > > > > if (page_ops & PAGE_UNLOCK) > > > > unlock_page(pages[i]); > > > > @@ -2550,7 +2556,7 @@ int end_extent_writepage(struct page *page, int err, > > u64 start, u64 end)> > > uptodate = 0; > > > > } > > > > - if (!uptodate) { > > + if (!uptodate || PageError(page)) { > > > > ClearPageUptodate(page); > > SetPageError(page); > > ret = ret < 0 ? ret : -EIO; > > > > @@ -3340,7 +3346,6 @@ static noinline_for_stack int > > writepage_delalloc(struct inode *inode,> > > nr_written); > > > > /* File system has been set read-only */ > > if (ret) { > > > > - SetPageError(page); > > > > /* fill_delalloc should be return < 0 for error > > > > * but just in case, we use > 0 here meaning the > > * IO is started, so we don't want to return > 0 > > > > @@ -3561,7 +3566,6 @@ static int __extent_writepage(struct page *page, > > struct writeback_control *wbc,> > > struct inode *inode = page->mapping->host; > > struct extent_page_data *epd = data; > > u64 start = page_offset(page); > > > > - u64 page_end = start + PAGE_CACHE_SIZE - 1; > > > > int ret; > > int nr = 0; > > size_t pg_offset = 0; > > > > @@ -3606,7 +3610,7 @@ static int __extent_writepage(struct page *page, > > struct writeback_control *wbc,> > > ret = writepage_delalloc(inode, page, wbc, epd, start, &nr_written); > > if (ret == 1) > > > > goto done_unlocked; > > > > - if (ret) > > + if (ret && !PagePrivate2(page)) > > > > goto done; > > > > ret = __extent_writepage_io(inode, page, wbc, epd, > > > > @@ -3620,10 +3624,7 @@ done: > > set_page_writeback(page); > > end_page_writeback(page); > > > > } > > > > - if (PageError(page)) { > > - ret = ret < 0 ? ret : -EIO; > > - end_extent_writepage(page, ret, start, page_end); > > - } > > + > > > > unlock_page(page); > > return ret; > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 8b4aaed..bff60c6 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -925,6 +925,8 @@ static noinline int cow_file_range(struct inode > > *inode, > > > > struct btrfs_key ins; > > struct extent_map *em; > > struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree; > > > > + struct btrfs_ordered_extent *ordered; > > + unsigned long page_ops, extent_ops; > > > > int ret = 0; > > > > if (btrfs_is_free_space_inode(inode)) { > > > > @@ -969,8 +971,6 @@ static noinline int cow_file_range(struct inode > > *inode, > > > > btrfs_drop_extent_cache(inode, start, start + num_bytes - 1, 0); > > > > while (disk_num_bytes > 0) { > > > > - unsigned long op; > > - > > > > cur_alloc_size = disk_num_bytes; > > ret = btrfs_reserve_extent(root, cur_alloc_size, > > > > root->sectorsize, 0, alloc_hint, > > > > @@ -1023,7 +1023,7 @@ static noinline int cow_file_range(struct inode > > *inode,> > > ret = btrfs_reloc_clone_csums(inode, start, > > > > cur_alloc_size); > > > > if (ret) > > > > - goto out_drop_extent_cache; > > + goto out_remove_ordered_extent; > > > > } > > > > if (disk_num_bytes < cur_alloc_size) > > > > @@ -1036,13 +1036,12 @@ static noinline int cow_file_range(struct inode > > *inode,> > > * Do set the Private2 bit so we know this page was properly > > * setup for writepage > > */ > > > > - op = unlock ? PAGE_UNLOCK : 0; > > - op |= PAGE_SET_PRIVATE2; > > - > > + page_ops = unlock ? PAGE_UNLOCK : 0; > > + page_ops |= PAGE_SET_PRIVATE2; > > + extent_ops = EXTENT_LOCKED | EXTENT_DELALLOC; > > > > extent_clear_unlock_delalloc(inode, start, > > > > - start + ram_size - 1, locked_page, > > - EXTENT_LOCKED | EXTENT_DELALLOC, > > - op); > > + start + ram_size - 1, locked_page, > > + extent_ops, page_ops); > > > > disk_num_bytes -= cur_alloc_size; > > num_bytes -= cur_alloc_size; > > alloc_hint = ins.objectid + ins.offset; > > > > @@ -1051,16 +1050,26 @@ static noinline int cow_file_range(struct inode > > *inode,> > > out: > > return ret; > > > > +out_remove_ordered_extent: > > + ordered = btrfs_lookup_ordered_extent(inode, ins.objectid); > > + BUG_ON(!ordered); > > + btrfs_remove_ordered_extent(inode, ordered); > > + > > Two problems here, > > 1. ins.objectid refers to block address while > btrfs_lookup_ordered_extent() expects a file offset. > Ah, That has most probably saved me from hours of debugging. Thanks a lot for pointing it out.
> 2. Removing ordered extent is not enough for cleaning it up, not only > this ordered extent remains in memory, but our reserved space number > needs to be cleaned up. > > If we have to do it this way, I'd copy what btrfs_finish_ordered_io() > does the cleanup job, however, I hope to call end_extent_writepage() > directly here to keep it simple as much as possible. > Yes, calling end_extent_writepage() looks to be the easiest and correct way to do this. I will test and include the change in the next version of the patchset. > Thanks, > > -liubo > > > out_drop_extent_cache: > > btrfs_drop_extent_cache(inode, start, start + ram_size - 1, 0); > > > > + > > > > out_reserve: > > btrfs_free_reserved_extent(root, ins.objectid, ins.offset, 1); > > > > + > > > > out_unlock: > > + page_ops = unlock ? PAGE_UNLOCK : 0; > > + page_ops |= PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK > > + | PAGE_SET_ERROR; > > + extent_ops = EXTENT_LOCKED | EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING > > + | EXTENT_DEFRAG; > > + > > > > extent_clear_unlock_delalloc(inode, start, end, locked_page, > > > > - EXTENT_LOCKED | EXTENT_DO_ACCOUNTING | > > - EXTENT_DELALLOC | EXTENT_DEFRAG, > > - PAGE_UNLOCK | PAGE_CLEAR_DIRTY | > > - PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK); > > + extent_ops, page_ops); > > > > goto out; > > > > } -- chandan -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
