On Thu, Jan 30, 2020 at 11:19 AM Nikolay Borisov <nborisov@xxxxxxxx> wrote:
>
>
>
> On 17.01.20 г. 16:02 ч., Josef Bacik wrote:
> > In order to keep track of where we have file extents on disk, and thus
> > where it is safe to adjust the i_size to, we need to have a tree in
> > place to keep track of the contiguous areas we have file extents for.
> > Add helpers to use this tree, as it's not required for NO_HOLES file
> > systems. We will use this by setting DIRTY for areas we know we have
> > file extent item's set, and clearing it when we remove file extent items
> > for truncation.
> >
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
> > ---
> > fs/btrfs/btrfs_inode.h | 5 +++
> > fs/btrfs/ctree.h | 5 +++
> > fs/btrfs/extent-io-tree.h | 1 +
> > fs/btrfs/extent_io.c | 11 +++++
> > fs/btrfs/file-item.c | 91 +++++++++++++++++++++++++++++++++++++++
> > fs/btrfs/inode.c | 6 +++
> > 6 files changed, 119 insertions(+)
> >
> > diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> > index 4e12a477d32e..d9dcbac513ed 100644
> > --- a/fs/btrfs/btrfs_inode.h
> > +++ b/fs/btrfs/btrfs_inode.h
> > @@ -60,6 +60,11 @@ struct btrfs_inode {
> > */
> > struct extent_io_tree io_failure_tree;
> >
> > + /* keeps track of where we have extent items mapped in order to make
> > + * sure our i_size adjustments are accurate.
> > + */
> > + struct extent_io_tree file_extent_tree;
> > +
> > /* held while logging the inode in tree-log.c */
> > struct mutex log_mutex;
> >
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index 00cf1641f1b9..8a2c1665baad 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -2851,6 +2851,11 @@ void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
> > struct btrfs_file_extent_item *fi,
> > const bool new_inline,
> > struct extent_map *em);
> > +int btrfs_inode_clear_file_extent_range(struct btrfs_inode *inode, u64 start,
> > + u64 len);
> > +int btrfs_inode_set_file_extent_range(struct btrfs_inode *inode, u64 start,
> > + u64 len);
> > +void btrfs_inode_safe_disk_i_size_write(struct inode *inode, u64 new_isize);
> >
> > /* inode.c */
> > struct extent_map *btrfs_get_extent_fiemap(struct btrfs_inode *inode,
> > diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
> > index a3febe746c79..c8bcd2e3184c 100644
> > --- a/fs/btrfs/extent-io-tree.h
> > +++ b/fs/btrfs/extent-io-tree.h
> > @@ -44,6 +44,7 @@ enum {
> > IO_TREE_TRANS_DIRTY_PAGES,
> > IO_TREE_ROOT_DIRTY_LOG_PAGES,
> > IO_TREE_SELFTEST,
> > + IO_TREE_INODE_FILE_EXTENT,
> > };
> >
> > struct extent_io_tree {
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index 0d40cd7427ba..f9b223d827b3 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -265,6 +265,15 @@ void __cold extent_io_exit(void)
> > bioset_exit(&btrfs_bioset);
> > }
> >
> > +/*
> > + * For the file_extent_tree, we want to hold the inode lock when we lookup and
> > + * update the disk_i_size, but lockdep will complain because our io_tree we hold
> > + * the tree lock and get the inode lock when setting delalloc. These two things
> > + * are unrelated, so make a class for the file_extent_tree so we don't get the
> > + * two locking patterns mixed up.
> > + */
> > +static struct lock_class_key file_extent_tree_class;
> > +
> > void extent_io_tree_init(struct btrfs_fs_info *fs_info,
> > struct extent_io_tree *tree, unsigned int owner,
> > void *private_data)
> > @@ -276,6 +285,8 @@ void extent_io_tree_init(struct btrfs_fs_info *fs_info,
> > spin_lock_init(&tree->lock);
> > tree->private_data = private_data;
> > tree->owner = owner;
> > + if (owner == IO_TREE_INODE_FILE_EXTENT)
> > + lockdep_set_class(&tree->lock, &file_extent_tree_class);
> > }
> >
> > void extent_io_tree_release(struct extent_io_tree *tree)
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index c2f365662d55..e5dc6c4b2f05 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -23,6 +23,97 @@
> > #define MAX_CSUM_ITEMS(r, size) (min_t(u32, __MAX_CSUM_ITEMS(r, size), \
> > PAGE_SIZE))
> >
> > +/**
> > + * @inode - the inode we want to update the disk_i_size for
> > + * @new_isize - the isize we want to set to, 0 if we use i_size
> > + *
> > + * With NO_HOLES set this simply sets the disk_is_size to whatever i_size_read()
> > + * returns as it is perfectly fine with a file that has holes without hole file
> > + * extent items.
> > + *
> > + * However for !NO_HOLES we need to only return the area that is contiguous from
> > + * the 0 offset of the file. Otherwise we could end up adjust i_size up to an
> > + * extent that has a gap in between.
> > + *
> > + * Finally new_isize should only be set in the case of truncate where we're not
> > + * ready to use i_size_read() as the limiter yet.
> > + */
> > +void btrfs_inode_safe_disk_i_size_write(struct inode *inode, u64 new_isize)
> > +{
> > + struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
> > + u64 start, end, isize;
> > + int ret;
> > +
> > + isize = new_isize ? new_isize : i_size_read(inode);
> > + if (btrfs_fs_incompat(fs_info, NO_HOLES)) {
> > + BTRFS_I(inode)->disk_i_size = isize;
> > + return;
> > + }
> > +
> > + spin_lock(&BTRFS_I(inode)->lock);
> > + ret = find_first_extent_bit(&BTRFS_I(inode)->file_extent_tree, 0,
> > + &start, &end, EXTENT_DIRTY, NULL);
> > + if (!ret && start == 0)
> > + isize = min(isize, end + 1);
> > + else
> > + isize = 0;
> > + BTRFS_I(inode)->disk_i_size = isize;
> > + spin_unlock(&BTRFS_I(inode)->lock);
> > +}
> > +
>
>
> What if we have the following layout for an inode:
>
> [----1M-EXTЕNT---][----2M-HOLE--------][------3M-EXTENT-----------]
>
> In this case the idisk size should be 4m whereas with the above code it
> will be 1M. Isn't this wrong?
No. That's precisely the point - to set it to 1Mb because there's an
implicit hole after it. For explicit holes, it will be 4M (that's done
in another patch in the series).
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
