lkml.org 
[lkml]   [2022]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] xfs: add missing cmap->br_state = XFS_EXT_NORM update
Hi Darrick,

On Thu, Feb 17, 2022 at 09:40:32PM -0800, Darrick J. Wong wrote:
> On Thu, Feb 17, 2022 at 05:55:42PM +0800, Gao Xiang wrote:
> > COW extents are already converted into written real extents after
> > xfs_reflink_convert_cow_locked(), therefore cmap->br_state should
> > reflect it.
> >
> > Otherwise, there is another necessary unwritten convertion
> > triggered in xfs_dio_write_end_io() for direct I/O cases.
> >
> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>
> I /think/ this looks ok. Does it test ok too? AFAICT nothing in the
> iomap/writeback machinery cares the incorrect state because we always
> set IOMAP_F_SHARED (which triggers COW) so I think this is simply a fix
> for directio, like you said?

Yeah, I think so, from the code logic buffered i/o seems no impacted...
And the unnecessary unwritten convertion under direct i/o takes
noticeable extra overhead in our workloads...

I checked my last night xfstests, it seems it stops unexpectedly (maybe
due to some environmental problem). I will rerun tests today and
feedback later.

Thanks,
Gao Xiang

>
> --D
>
> > ---
> > From the logic itself and runtime tracing, IMO, it seems true.
> > Kindly correct me here if my understanding is wrong.
> >
> > fs/xfs/xfs_reflink.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 75bd2e03cd5b..5f0a364739a5 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -424,7 +424,10 @@ xfs_reflink_allocate_cow(
> > if (!convert_now || cmap->br_state == XFS_EXT_NORM)
> > return 0;
> > trace_xfs_reflink_convert_cow(ip, cmap);
> > - return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> > + error = xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> > + if (!error)
> > + cmap->br_state = XFS_EXT_NORM;
> > + return error;
> >
> > out_trans_cancel:
> > xfs_trans_cancel(tp);
> > --
> > 2.24.4
> >

\
 
 \ /
  Last update: 2022-02-18 06:56    [W:0.072 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site