mbox series

[00/14] Another batch of inode vs btrfs_inode cleanups

Message ID 20201102144906.3767963-1-nborisov@suse.com (mailing list archive)
Headers show
Series Another batch of inode vs btrfs_inode cleanups | expand

Message

Nikolay Borisov Nov. 2, 2020, 2:48 p.m. UTC
Here is another batch which  gets use closer to unified btrfs_inode vs inode
usage in functions.

Nikolay Borisov (14):
  btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
  btrfs: Make insert_prealloc_file_extent take btrfs_inode
  btrfs: Make btrfs_truncate_inode_items take btrfs_inode
  btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
  btrfs: Make btrfs_delayed_update_inode take btrfs_inode
  btrfs: Make btrfs_update_inode_item take btrfs_inode
  btrfs: Make btrfs_update_inode take btrfs_inode
  btrfs: Make maybe_insert_hole take btrfs_inode
  btrfs: Make find_first_non_hole take btrfs_inode
  btrfs: Make btrfs_insert_replace_extent take btrfs_inode
  btrfs: Make btrfs_truncate_block take btrfs_inode
  btrfs: Make btrfs_cont_expand take btrfs_inode
  btrfs: Make btrfs_drop_extents take btrfs_inode
  btrfs: Make btrfs_update_inode_fallback take btrfs_inode

 fs/btrfs/block-group.c      |   2 +-
 fs/btrfs/ctree.h            |  21 +--
 fs/btrfs/delayed-inode.c    |  13 +-
 fs/btrfs/delayed-inode.h    |   3 +-
 fs/btrfs/file-item.c        |  18 +--
 fs/btrfs/file.c             |  88 +++++++------
 fs/btrfs/free-space-cache.c |   8 +-
 fs/btrfs/inode-map.c        |   2 +-
 fs/btrfs/inode.c            | 249 ++++++++++++++++++------------------
 fs/btrfs/ioctl.c            |   6 +-
 fs/btrfs/reflink.c          |   9 +-
 fs/btrfs/transaction.c      |   3 +-
 fs/btrfs/tree-log.c         |  24 ++--
 fs/btrfs/xattr.c            |   4 +-
 14 files changed, 233 insertions(+), 217 deletions(-)

--
2.25.1

Comments

Johannes Thumshirn Nov. 2, 2020, 3:48 p.m. UTC | #1
On 02/11/2020 15:49, Nikolay Borisov wrote:
> Here is another batch which  gets use closer to unified btrfs_inode vs inode
> usage in functions.
> 
> Nikolay Borisov (14):
>   btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
>   btrfs: Make insert_prealloc_file_extent take btrfs_inode
>   btrfs: Make btrfs_truncate_inode_items take btrfs_inode
>   btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
>   btrfs: Make btrfs_delayed_update_inode take btrfs_inode
>   btrfs: Make btrfs_update_inode_item take btrfs_inode
>   btrfs: Make btrfs_update_inode take btrfs_inode
>   btrfs: Make maybe_insert_hole take btrfs_inode
>   btrfs: Make find_first_non_hole take btrfs_inode
>   btrfs: Make btrfs_insert_replace_extent take btrfs_inode
>   btrfs: Make btrfs_truncate_block take btrfs_inode
>   btrfs: Make btrfs_cont_expand take btrfs_inode
>   btrfs: Make btrfs_drop_extents take btrfs_inode
>   btrfs: Make btrfs_update_inode_fallback take btrfs_inode
> 
>  fs/btrfs/block-group.c      |   2 +-
>  fs/btrfs/ctree.h            |  21 +--
>  fs/btrfs/delayed-inode.c    |  13 +-
>  fs/btrfs/delayed-inode.h    |   3 +-
>  fs/btrfs/file-item.c        |  18 +--
>  fs/btrfs/file.c             |  88 +++++++------
>  fs/btrfs/free-space-cache.c |   8 +-
>  fs/btrfs/inode-map.c        |   2 +-
>  fs/btrfs/inode.c            | 249 ++++++++++++++++++------------------
>  fs/btrfs/ioctl.c            |   6 +-
>  fs/btrfs/reflink.c          |   9 +-
>  fs/btrfs/transaction.c      |   3 +-
>  fs/btrfs/tree-log.c         |  24 ++--
>  fs/btrfs/xattr.c            |   4 +-
>  14 files changed, 233 insertions(+), 217 deletions(-)
> 
> --
> 2.25.1
> 
> 


Code wise this looks good to me. FYI patches 11/14 and 12/14 don't 
apply cleanly on today's misc-next (at least for me).

For the whole series:
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
David Sterba Nov. 2, 2020, 4:10 p.m. UTC | #2
On Mon, Nov 02, 2020 at 03:48:52PM +0000, Johannes Thumshirn wrote:
> On 02/11/2020 15:49, Nikolay Borisov wrote:
> > Here is another batch which  gets use closer to unified btrfs_inode vs inode
> > usage in functions.
> > 
> > Nikolay Borisov (14):
> >   btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
> >   btrfs: Make insert_prealloc_file_extent take btrfs_inode
> >   btrfs: Make btrfs_truncate_inode_items take btrfs_inode
> >   btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
> >   btrfs: Make btrfs_delayed_update_inode take btrfs_inode
> >   btrfs: Make btrfs_update_inode_item take btrfs_inode
> >   btrfs: Make btrfs_update_inode take btrfs_inode
> >   btrfs: Make maybe_insert_hole take btrfs_inode
> >   btrfs: Make find_first_non_hole take btrfs_inode
> >   btrfs: Make btrfs_insert_replace_extent take btrfs_inode
> >   btrfs: Make btrfs_truncate_block take btrfs_inode
> >   btrfs: Make btrfs_cont_expand take btrfs_inode
> >   btrfs: Make btrfs_drop_extents take btrfs_inode
> >   btrfs: Make btrfs_update_inode_fallback take btrfs_inode
> > 
> >  fs/btrfs/block-group.c      |   2 +-
> >  fs/btrfs/ctree.h            |  21 +--
> >  fs/btrfs/delayed-inode.c    |  13 +-
> >  fs/btrfs/delayed-inode.h    |   3 +-
> >  fs/btrfs/file-item.c        |  18 +--
> >  fs/btrfs/file.c             |  88 +++++++------
> >  fs/btrfs/free-space-cache.c |   8 +-
> >  fs/btrfs/inode-map.c        |   2 +-
> >  fs/btrfs/inode.c            | 249 ++++++++++++++++++------------------
> >  fs/btrfs/ioctl.c            |   6 +-
> >  fs/btrfs/reflink.c          |   9 +-
> >  fs/btrfs/transaction.c      |   3 +-
> >  fs/btrfs/tree-log.c         |  24 ++--
> >  fs/btrfs/xattr.c            |   4 +-
> >  14 files changed, 233 insertions(+), 217 deletions(-)
> > 
> > --
> > 2.25.1
> > 
> > 
> 
> 
> Code wise this looks good to me. FYI patches 11/14 and 12/14 don't 
> apply cleanly on today's misc-next (at least for me).

We're expecting conflicts, there are about 50+ patches in for-next where
it could collide but I'd like to see what's the scope so I could
better schedule the order.
Johannes Thumshirn Nov. 2, 2020, 4:25 p.m. UTC | #3
On 02/11/2020 17:12, David Sterba wrote:
> On Mon, Nov 02, 2020 at 03:48:52PM +0000, Johannes Thumshirn wrote:
>> On 02/11/2020 15:49, Nikolay Borisov wrote:
>>> Here is another batch which  gets use closer to unified btrfs_inode vs inode
>>> usage in functions.
>>>
>>> Nikolay Borisov (14):
>>>   btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
>>>   btrfs: Make insert_prealloc_file_extent take btrfs_inode
>>>   btrfs: Make btrfs_truncate_inode_items take btrfs_inode
>>>   btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
>>>   btrfs: Make btrfs_delayed_update_inode take btrfs_inode
>>>   btrfs: Make btrfs_update_inode_item take btrfs_inode
>>>   btrfs: Make btrfs_update_inode take btrfs_inode
>>>   btrfs: Make maybe_insert_hole take btrfs_inode
>>>   btrfs: Make find_first_non_hole take btrfs_inode
>>>   btrfs: Make btrfs_insert_replace_extent take btrfs_inode
>>>   btrfs: Make btrfs_truncate_block take btrfs_inode
>>>   btrfs: Make btrfs_cont_expand take btrfs_inode
>>>   btrfs: Make btrfs_drop_extents take btrfs_inode
>>>   btrfs: Make btrfs_update_inode_fallback take btrfs_inode
>>>
>>>  fs/btrfs/block-group.c      |   2 +-
>>>  fs/btrfs/ctree.h            |  21 +--
>>>  fs/btrfs/delayed-inode.c    |  13 +-
>>>  fs/btrfs/delayed-inode.h    |   3 +-
>>>  fs/btrfs/file-item.c        |  18 +--
>>>  fs/btrfs/file.c             |  88 +++++++------
>>>  fs/btrfs/free-space-cache.c |   8 +-
>>>  fs/btrfs/inode-map.c        |   2 +-
>>>  fs/btrfs/inode.c            | 249 ++++++++++++++++++------------------
>>>  fs/btrfs/ioctl.c            |   6 +-
>>>  fs/btrfs/reflink.c          |   9 +-
>>>  fs/btrfs/transaction.c      |   3 +-
>>>  fs/btrfs/tree-log.c         |  24 ++--
>>>  fs/btrfs/xattr.c            |   4 +-
>>>  14 files changed, 233 insertions(+), 217 deletions(-)
>>>
>>> --
>>> 2.25.1
>>>
>>>
>>
>>
>> Code wise this looks good to me. FYI patches 11/14 and 12/14 don't 
>> apply cleanly on today's misc-next (at least for me).
> 
> We're expecting conflicts, there are about 50+ patches in for-next where
> it could collide but I'd like to see what's the scope so I could
> better schedule the order.
> 

For 11:

error: while searching for:
        unlock_extent_cached(io_tree, block_start, block_end, &cached_state);

        if (only_release_metadata)
                set_extent_bit(&BTRFS_I(inode)->io_tree, block_start,
                                block_end, EXTENT_NORESERVE, NULL, NULL,
                                          one NULL too much ~~^
                                GFP_NOFS);

out_unlock:
        if (ret) {
                if (only_release_metadata)
                        btrfs_delalloc_release_metadata(BTRFS_I(inode),
                                        blocksize, true);
                else
                        btrfs_delalloc_release_space(BTRFS_I(inode), data_reserved,
                                        block_start, blocksize, true);
        }
        btrfs_delalloc_release_extents(BTRFS_I(inode), blocksize);
        unlock_page(page);
        put_page(page);
out:
        if (only_release_metadata)
                btrfs_check_nocow_unlock(BTRFS_I(inode));
        extent_changeset_free(data_reserved);
        return ret;
}

error: patch failed: fs/btrfs/inode.c:4601
Hunk #7 succeeded at 4763 (offset 70 lines).
Hunk #8 succeeded at 8428 (offset -68 lines).
Applied patch fs/btrfs/ctree.h cleanly.
Applied patch fs/btrfs/file.c cleanly.
Applying patch fs/btrfs/inode.c with 1 reject...
Hunk #1 applied cleanly.
Hunk #2 applied cleanly.
Hunk #3 applied cleanly.
Hunk #4 applied cleanly.
Hunk #5 applied cleanly.
Rejected hunk #6.
Hunk #7 applied cleanly.
Hunk #8 applied cleanly.
Patch failed at 0011 btrfs: Make btrfs_truncate_block take btrfs_inode

For 12:
Hunk #1 succeeded at 3049 (offset 5 lines).
Checking patch fs/btrfs/file.c...
error: while searching for:
                /* Expand hole size to cover write data, preventing empty gap */
                end_pos = round_up(pos + count,
                                   fs_info->sectorsize);

No linebreak after the comma but a blank line in misc-next

                err = btrfs_cont_expand(inode, oldsize, end_pos);
                if (err) {
                        inode_unlock(inode);
                        goto out;

error: patch failed: fs/btrfs/file.c:1989
Hunk #2 succeeded at 3382 (offset 15 lines).
Checking patch fs/btrfs/inode.c...
Hunk #1 succeeded at 4742 (offset 69 lines).
Hunk #2 succeeded at 4762 (offset 69 lines).
Hunk #3 succeeded at 4787 (offset 69 lines).
Hunk #4 succeeded at 4822 (offset 69 lines).
Hunk #5 succeeded at 4876 (offset 69 lines).
Checking patch fs/btrfs/reflink.c...
Applied patch fs/btrfs/ctree.h cleanly.
Applying patch fs/btrfs/file.c with 1 reject...
Rejected hunk #1.
Hunk #2 applied cleanly.
Applied patch fs/btrfs/inode.c cleanly.
Applied patch fs/btrfs/reflink.c cleanly.
Patch failed at 0012 btrfs: Make btrfs_cont_expand take btrfs_inode

Overall easy to fixup when applying or rebasing
David Sterba Nov. 5, 2020, 4:21 p.m. UTC | #4
On Mon, Nov 02, 2020 at 04:48:52PM +0200, Nikolay Borisov wrote:
> Here is another batch which  gets use closer to unified btrfs_inode vs inode
> usage in functions.
> 
> Nikolay Borisov (14):
>   btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
>   btrfs: Make insert_prealloc_file_extent take btrfs_inode
>   btrfs: Make btrfs_truncate_inode_items take btrfs_inode
>   btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
>   btrfs: Make btrfs_delayed_update_inode take btrfs_inode
>   btrfs: Make btrfs_update_inode_item take btrfs_inode
>   btrfs: Make btrfs_update_inode take btrfs_inode
>   btrfs: Make maybe_insert_hole take btrfs_inode
>   btrfs: Make find_first_non_hole take btrfs_inode
>   btrfs: Make btrfs_insert_replace_extent take btrfs_inode
>   btrfs: Make btrfs_truncate_block take btrfs_inode
>   btrfs: Make btrfs_cont_expand take btrfs_inode
>   btrfs: Make btrfs_drop_extents take btrfs_inode
>   btrfs: Make btrfs_update_inode_fallback take btrfs_inode

With a few fixups it's now in a topic branch and it does not conflict
with the pending branches. I will postpone merging it until we have the
core changes in (like the preemptive flushing and such) but so far it
seems to be smoother than expected. Thanks.
David Sterba Nov. 12, 2020, 6:25 p.m. UTC | #5
On Thu, Nov 05, 2020 at 05:21:12PM +0100, David Sterba wrote:
> On Mon, Nov 02, 2020 at 04:48:52PM +0200, Nikolay Borisov wrote:
> > Here is another batch which  gets use closer to unified btrfs_inode vs inode
> > usage in functions.
> > 
> > Nikolay Borisov (14):
> >   btrfs: Make btrfs_inode_safe_disk_i_size_write take btrfs_inode
> >   btrfs: Make insert_prealloc_file_extent take btrfs_inode
> >   btrfs: Make btrfs_truncate_inode_items take btrfs_inode
> >   btrfs: Make btrfs_finish_ordered_io btrfs_inode-centric
> >   btrfs: Make btrfs_delayed_update_inode take btrfs_inode
> >   btrfs: Make btrfs_update_inode_item take btrfs_inode
> >   btrfs: Make btrfs_update_inode take btrfs_inode
> >   btrfs: Make maybe_insert_hole take btrfs_inode
> >   btrfs: Make find_first_non_hole take btrfs_inode
> >   btrfs: Make btrfs_insert_replace_extent take btrfs_inode
> >   btrfs: Make btrfs_truncate_block take btrfs_inode
> >   btrfs: Make btrfs_cont_expand take btrfs_inode
> >   btrfs: Make btrfs_drop_extents take btrfs_inode
> >   btrfs: Make btrfs_update_inode_fallback take btrfs_inode
> 
> With a few fixups it's now in a topic branch and it does not conflict
> with the pending branches. I will postpone merging it until we have the
> core changes in (like the preemptive flushing and such) but so far it
> seems to be smoother than expected. Thanks.

Moved to misc-next.