Re: [PATCH 13/20] ext4: get rid of ppath in ext4_force_split_extent_at()

[Baokun Li]

Hi Ojaswin,

On 2024/8/3 4:01, Ojaswin Mujoo wrote:
> On Wed, Jul 10, 2024 at 12:06:47PM +0800, libaokun@xxxxxxxxxxxxxxx wrote:
> > From: Baokun Li <libaokun1@xxxxxxxxxx>
> >
> > The use of path and ppath is now very confusing, so to make the code more
> > readable, pass path between functions uniformly, and get rid of ppath.
> >
> > To get rid of the ppath in ext4_force_split_extent_at(), the following is
> > done here:
> >
> >   * The ext4_find_extent() can update the extent path so it doesn't have to
> >     allocate and free path repeatedly, thus reducing the consumption of
> >     memory allocation and freeing in ext4_swap_extents().
> >
> > No functional changes.
> Looks good Baokun, feel free to add:
>
> Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
>
> One small comment below..
Thanks for the review!
> > Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
> > ---
> >   fs/ext4/extents.c | 117 ++++++++++++++++++++++++----------------------
> >   1 file changed, 60 insertions(+), 57 deletions(-)
> >
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index c86b1bb7720f..0bd068ed324f 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> .. snip ..
>
> > @@ -5707,25 +5701,21 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
> In ext4_swap_extents, maybe we should keep the refactoring to a separate
> patch than the changes needed to get rid of ppath in
> ext4_force_split_extent_at(), the commits would look a bit cleaner and
> easier to read that way. I don't feel too strongly about it tho and
> I'll let you take a call.
>
> Regards,
> ojaswin
Totally agree! It does seem a bit unclear now to put the refactoring
of ext4_swap_extents() into a modification to get rid of the ppath.
In the next version I will put the logic for refactoring
ext4_swap_extents() into a separate patch.

Cheers,
Baokun