On Mon, Aug 24, 2020 at 04:30:52PM -0400, Josef Bacik wrote: > On 8/21/20 3:38 AM, Omar Sandoval wrote: > > From: Omar Sandoval <osandov@xxxxxx> > > > > The implementation resembles direct I/O: we have to flush any ordered > > extents, invalidate the page cache, and do the io tree/delalloc/extent > > map/ordered extent dance. From there, we can reuse the compression code > > with a minor modification to distinguish the write from writeback. This > > also creates inline extents when possible. > > > > Now that read and write are implemented, this also sets the > > FMODE_ENCODED_IO flag in btrfs_file_open(). > > > > Signed-off-by: Omar Sandoval <osandov@xxxxxx> > > --- > > fs/btrfs/compression.c | 7 +- > > fs/btrfs/compression.h | 6 +- > > fs/btrfs/ctree.h | 2 + > > fs/btrfs/file.c | 40 +++++-- > > fs/btrfs/inode.c | 246 +++++++++++++++++++++++++++++++++++++++- > > fs/btrfs/ordered-data.c | 12 +- > > fs/btrfs/ordered-data.h | 2 + > > 7 files changed, 298 insertions(+), 17 deletions(-) > > > > <snip> > > > + > > + ret = btrfs_alloc_data_chunk_ondemand(BTRFS_I(inode), disk_num_bytes); > > + if (ret) > > + goto out_unlock; > > + ret = btrfs_qgroup_reserve_data(BTRFS_I(inode), &data_reserved, start, > > + num_bytes); > > + if (ret) > > + goto out_free_data_space; > > + ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), num_bytes, > > + disk_num_bytes); > > + if (ret) > > + goto out_qgroup_free_data; > > This can just be btrfs_delalloc_reserve_space() and that way the error > handling is much cleaner. > > <snip> > > + > > +out_free_reserved: > > + btrfs_dec_block_group_reservations(fs_info, ins.objectid); > > + btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 1); > > +out_delalloc_release: > > + btrfs_delalloc_release_extents(BTRFS_I(inode), num_bytes); > > + btrfs_delalloc_release_metadata(BTRFS_I(inode), disk_num_bytes, > > + ret < 0); > > Likewise this can all just be btrfs_free_reserved_data_space(). Thanks, > > Josef btrfs_delalloc_reserve_space() and btrfs_free_reserved_data_space() assume that num_bytes == disk_num_bytes, which isn't true for RWF_ENCODED. I figured it'd be cleaner to open-code this special case in the one place that it's needed, but I could also add explicit num_bytes and disk_num_bytes arguments to btrfs_delalloc_reserve_space() and btrfs_free_reserved_data_space(). They'd just be equal everywhere except for here. If you're fine with keeping it this way, I'll add a comment explaining why we can't use the higher-level helpers.
