On Fri, Jun 28, 2019 at 08:32:15AM +0300, Nikolay Borisov wrote: > > > On 27.06.19 г. 20:00 ч., fdmanana@xxxxxxxxxx wrote: > > From: Filipe Manana <fdmanana@xxxxxxxx> > > > > Move the code that is responsible for dropping extents in a range out of > > btrfs_punch_hole() into a new helper function, btrfs_punch_hole_range(), > > so that later it can be used by the reflinking (extent cloning and dedup) > > code to fix a ENOSPC bug. > > > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx> > > --- > > fs/btrfs/file.c | 308 ++++++++++++++++++++++++++++++-------------------------- > > 1 file changed, 166 insertions(+), 142 deletions(-) > > > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > > index 1c7533db16b0..393a6d23b6b0 100644 > > --- a/fs/btrfs/file.c > > +++ b/fs/btrfs/file.c > > @@ -2448,27 +2448,171 @@ static int btrfs_punch_hole_lock_range(struct inode *inode, > > return 0; > > } > > > > +/* > > + * The respective range must have been previously locked, as well as the inode. > > + * The end offset is inclusive (last byte of the range). > > + */ > > +static int btrfs_punch_hole_range(struct inode *inode, struct btrfs_path *path, > > + const u64 start, const u64 end, > > + struct btrfs_trans_handle **trans_out) > > I'm not a big fan of the way a lower function starts a transaction which > is then passed to the caller. So while it fixes a real bug in the next > patch it isn't really pushing the code in the right direction. I see > that this transaction is bound to whether no_hole is enabled or not so > it's not just a matter of lifting it up to the caller. And there's also > the while loop which commits it and starts a new one. So yeah, it > doesn't seem like there's a significantly better way of doing that now > but IMO we need to think of cleaning that up later. I agree with the point about starting the transaction from the lower function, and also don't see a better way to do it right now. Oh well.
