* [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow @ 2019-02-24 20:30 Darrick J. Wong 2019-02-24 20:31 ` [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin Darrick J. Wong 2019-02-25 12:55 ` [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Brian Foster 0 siblings, 2 replies; 7+ messages in thread From: Darrick J. Wong @ 2019-02-24 20:30 UTC (permalink / raw) To: darrick.wong; +Cc: linux-xfs, bfoster From: Darrick J. Wong <darrick.wong@oracle.com> Don't pass raw iomap flags to xfs_reflink_allocate_cow; signal our intention with a boolean argument. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/xfs/xfs_iomap.c | 6 +++--- fs/xfs/xfs_reflink.c | 4 ++-- fs/xfs/xfs_reflink.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 01210eae8bf3..a376fb2d4b94 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -983,6 +983,7 @@ xfs_file_iomap_begin( */ if (xfs_is_cow_inode(ip)) { struct xfs_bmbt_irec orig = imap; + bool convert_now = (flags & IOMAP_DIRECT); /* if zeroing doesn't need COW allocation, then we are done. */ if ((flags & IOMAP_ZERO) && @@ -991,7 +992,7 @@ xfs_file_iomap_begin( /* may drop and re-acquire the ilock */ error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, - flags); + convert_now); if (error) goto out_unlock; @@ -1002,8 +1003,7 @@ xfs_file_iomap_begin( * direct I/O code, which must be block aligned we need to * report the newly allocated address. */ - if (!(flags & IOMAP_DIRECT) && - orig.br_startblock != HOLESTARTBLOCK) + if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) imap = orig; end_fsb = imap.br_startoff + imap.br_blockcount; diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index e2d9179bd50d..d42e3ef9050e 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -361,7 +361,7 @@ xfs_reflink_allocate_cow( struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, - unsigned iomap_flags) + bool convert_now) { struct xfs_mount *mp = ip->i_mount; xfs_fileoff_t offset_fsb = imap->br_startoff; @@ -444,7 +444,7 @@ xfs_reflink_allocate_cow( * to initiate a disk write. For direct I/O we are going to write the * data and need the conversion, but for buffered writes we're done. */ - if (!(iomap_flags & IOMAP_DIRECT) || imap->br_state == XFS_EXT_NORM) + if (!convert_now || imap->br_state == XFS_EXT_NORM) return 0; trace_xfs_reflink_convert_cow(ip, imap); return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb); diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h index 2a3052fbe23e..28a43b7f581d 100644 --- a/fs/xfs/xfs_reflink.h +++ b/fs/xfs/xfs_reflink.h @@ -27,7 +27,7 @@ bool xfs_inode_need_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, extern int xfs_reflink_allocate_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, - unsigned iomap_flags); + bool convert_now); extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset, xfs_off_t count); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin 2019-02-24 20:30 [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Darrick J. Wong @ 2019-02-24 20:31 ` Darrick J. Wong 2019-02-25 12:55 ` Brian Foster 2019-02-25 12:55 ` [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Brian Foster 1 sibling, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2019-02-24 20:31 UTC (permalink / raw) To: darrick.wong; +Cc: linux-xfs, bfoster From: Darrick J. Wong <darrick.wong@oracle.com> Rework the data flow in xfs_file_iomap_begin where we decide if we have to break shared extents. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/xfs/xfs_iomap.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index a376fb2d4b94..a3043b1bcd9a 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -982,7 +982,7 @@ xfs_file_iomap_begin( * been done up front, so we don't need to do them here. */ if (xfs_is_cow_inode(ip)) { - struct xfs_bmbt_irec orig = imap; + struct xfs_bmbt_irec cmap; bool convert_now = (flags & IOMAP_DIRECT); /* if zeroing doesn't need COW allocation, then we are done. */ @@ -991,7 +991,8 @@ xfs_file_iomap_begin( goto out_found; /* may drop and re-acquire the ilock */ - error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, + cmap = imap; + error = xfs_reflink_allocate_cow(ip, &cmap, &shared, &lockmode, convert_now); if (error) goto out_unlock; @@ -1001,10 +1002,12 @@ xfs_file_iomap_begin( * previous block (if there was any) so that the higher level * write code can perform read-modify-write operations. For * direct I/O code, which must be block aligned we need to - * report the newly allocated address. + * report the newly allocated address. If the data fork has + * a hole, copy the COW fork mapping to avoid allocating to + * the data fork. */ - if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) - imap = orig; + if (convert_now || imap.br_startblock == HOLESTARTBLOCK) + imap = cmap; end_fsb = imap.br_startoff + imap.br_blockcount; length = XFS_FSB_TO_B(mp, end_fsb) - offset; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin 2019-02-24 20:31 ` [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin Darrick J. Wong @ 2019-02-25 12:55 ` Brian Foster 0 siblings, 0 replies; 7+ messages in thread From: Brian Foster @ 2019-02-25 12:55 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On Sun, Feb 24, 2019 at 12:31:05PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Rework the data flow in xfs_file_iomap_begin where we decide if we have > to break shared extents. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_iomap.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index a376fb2d4b94..a3043b1bcd9a 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -982,7 +982,7 @@ xfs_file_iomap_begin( > * been done up front, so we don't need to do them here. > */ > if (xfs_is_cow_inode(ip)) { > - struct xfs_bmbt_irec orig = imap; > + struct xfs_bmbt_irec cmap; > bool convert_now = (flags & IOMAP_DIRECT); > > /* if zeroing doesn't need COW allocation, then we are done. */ > @@ -991,7 +991,8 @@ xfs_file_iomap_begin( > goto out_found; > > /* may drop and re-acquire the ilock */ > - error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, > + cmap = imap; > + error = xfs_reflink_allocate_cow(ip, &cmap, &shared, &lockmode, > convert_now); > if (error) > goto out_unlock; > @@ -1001,10 +1002,12 @@ xfs_file_iomap_begin( > * previous block (if there was any) so that the higher level > * write code can perform read-modify-write operations. For > * direct I/O code, which must be block aligned we need to > - * report the newly allocated address. > + * report the newly allocated address. If the data fork has > + * a hole, copy the COW fork mapping to avoid allocating to > + * the data fork. > */ > - if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) > - imap = orig; > + if (convert_now || imap.br_startblock == HOLESTARTBLOCK) > + imap = cmap; > > end_fsb = imap.br_startoff + imap.br_blockcount; > length = XFS_FSB_TO_B(mp, end_fsb) - offset; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow 2019-02-24 20:30 [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Darrick J. Wong 2019-02-24 20:31 ` [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin Darrick J. Wong @ 2019-02-25 12:55 ` Brian Foster 2019-02-25 16:59 ` Darrick J. Wong 1 sibling, 1 reply; 7+ messages in thread From: Brian Foster @ 2019-02-25 12:55 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On Sun, Feb 24, 2019 at 12:30:58PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Don't pass raw iomap flags to xfs_reflink_allocate_cow; signal our > intention with a boolean argument. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_iomap.c | 6 +++--- > fs/xfs/xfs_reflink.c | 4 ++-- > fs/xfs/xfs_reflink.h | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index 01210eae8bf3..a376fb2d4b94 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -983,6 +983,7 @@ xfs_file_iomap_begin( > */ > if (xfs_is_cow_inode(ip)) { > struct xfs_bmbt_irec orig = imap; > + bool convert_now = (flags & IOMAP_DIRECT); > > /* if zeroing doesn't need COW allocation, then we are done. */ > if ((flags & IOMAP_ZERO) && > @@ -991,7 +992,7 @@ xfs_file_iomap_begin( > > /* may drop and re-acquire the ilock */ > error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, > - flags); > + convert_now); > if (error) > goto out_unlock; > > @@ -1002,8 +1003,7 @@ xfs_file_iomap_begin( > * direct I/O code, which must be block aligned we need to > * report the newly allocated address. > */ > - if (!(flags & IOMAP_DIRECT) && > - orig.br_startblock != HOLESTARTBLOCK) > + if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) FWIW, I thought the original dio check made sense here (in the iomap callback), whereas the flags parameter to the external function seemed more of a layering hack. Looks fine, anyways: Reviewed-by: Brian Foster <bfoster@redhat.com> > imap = orig; > > end_fsb = imap.br_startoff + imap.br_blockcount; > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index e2d9179bd50d..d42e3ef9050e 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -361,7 +361,7 @@ xfs_reflink_allocate_cow( > struct xfs_bmbt_irec *imap, > bool *shared, > uint *lockmode, > - unsigned iomap_flags) > + bool convert_now) > { > struct xfs_mount *mp = ip->i_mount; > xfs_fileoff_t offset_fsb = imap->br_startoff; > @@ -444,7 +444,7 @@ xfs_reflink_allocate_cow( > * to initiate a disk write. For direct I/O we are going to write the > * data and need the conversion, but for buffered writes we're done. > */ > - if (!(iomap_flags & IOMAP_DIRECT) || imap->br_state == XFS_EXT_NORM) > + if (!convert_now || imap->br_state == XFS_EXT_NORM) > return 0; > trace_xfs_reflink_convert_cow(ip, imap); > return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb); > diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h > index 2a3052fbe23e..28a43b7f581d 100644 > --- a/fs/xfs/xfs_reflink.h > +++ b/fs/xfs/xfs_reflink.h > @@ -27,7 +27,7 @@ bool xfs_inode_need_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, > > extern int xfs_reflink_allocate_cow(struct xfs_inode *ip, > struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, > - unsigned iomap_flags); > + bool convert_now); > extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset, > xfs_off_t count); > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow 2019-02-25 12:55 ` [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Brian Foster @ 2019-02-25 16:59 ` Darrick J. Wong 2019-02-25 17:18 ` Brian Foster 0 siblings, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2019-02-25 16:59 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs On Mon, Feb 25, 2019 at 07:55:02AM -0500, Brian Foster wrote: > On Sun, Feb 24, 2019 at 12:30:58PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Don't pass raw iomap flags to xfs_reflink_allocate_cow; signal our > > intention with a boolean argument. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > fs/xfs/xfs_iomap.c | 6 +++--- > > fs/xfs/xfs_reflink.c | 4 ++-- > > fs/xfs/xfs_reflink.h | 2 +- > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > > index 01210eae8bf3..a376fb2d4b94 100644 > > --- a/fs/xfs/xfs_iomap.c > > +++ b/fs/xfs/xfs_iomap.c > > @@ -983,6 +983,7 @@ xfs_file_iomap_begin( > > */ > > if (xfs_is_cow_inode(ip)) { > > struct xfs_bmbt_irec orig = imap; > > + bool convert_now = (flags & IOMAP_DIRECT); > > > > /* if zeroing doesn't need COW allocation, then we are done. */ > > if ((flags & IOMAP_ZERO) && > > @@ -991,7 +992,7 @@ xfs_file_iomap_begin( > > > > /* may drop and re-acquire the ilock */ > > error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, > > - flags); > > + convert_now); > > if (error) > > goto out_unlock; > > > > @@ -1002,8 +1003,7 @@ xfs_file_iomap_begin( > > * direct I/O code, which must be block aligned we need to > > * report the newly allocated address. > > */ > > - if (!(flags & IOMAP_DIRECT) && > > - orig.br_startblock != HOLESTARTBLOCK) > > + if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) > > FWIW, I thought the original dio check made sense here (in the iomap > callback), whereas the flags parameter to the external function seemed > more of a layering hack. Looks fine, anyways: Yeah, I went back and forth about this 'convert_now'. The flag is trying to convey that we need to return the disk location of where we want the write to land. bool need_real_write_location = (flags & IOMAP_DIRECT); But that's a long name. "need_write_map", perhaps? --D > Reviewed-by: Brian Foster <bfoster@redhat.com> > > > imap = orig; > > > > end_fsb = imap.br_startoff + imap.br_blockcount; > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > > index e2d9179bd50d..d42e3ef9050e 100644 > > --- a/fs/xfs/xfs_reflink.c > > +++ b/fs/xfs/xfs_reflink.c > > @@ -361,7 +361,7 @@ xfs_reflink_allocate_cow( > > struct xfs_bmbt_irec *imap, > > bool *shared, > > uint *lockmode, > > - unsigned iomap_flags) > > + bool convert_now) > > { > > struct xfs_mount *mp = ip->i_mount; > > xfs_fileoff_t offset_fsb = imap->br_startoff; > > @@ -444,7 +444,7 @@ xfs_reflink_allocate_cow( > > * to initiate a disk write. For direct I/O we are going to write the > > * data and need the conversion, but for buffered writes we're done. > > */ > > - if (!(iomap_flags & IOMAP_DIRECT) || imap->br_state == XFS_EXT_NORM) > > + if (!convert_now || imap->br_state == XFS_EXT_NORM) > > return 0; > > trace_xfs_reflink_convert_cow(ip, imap); > > return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb); > > diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h > > index 2a3052fbe23e..28a43b7f581d 100644 > > --- a/fs/xfs/xfs_reflink.h > > +++ b/fs/xfs/xfs_reflink.h > > @@ -27,7 +27,7 @@ bool xfs_inode_need_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, > > > > extern int xfs_reflink_allocate_cow(struct xfs_inode *ip, > > struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, > > - unsigned iomap_flags); > > + bool convert_now); > > extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset, > > xfs_off_t count); > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow 2019-02-25 16:59 ` Darrick J. Wong @ 2019-02-25 17:18 ` Brian Foster 2019-02-25 17:43 ` Darrick J. Wong 0 siblings, 1 reply; 7+ messages in thread From: Brian Foster @ 2019-02-25 17:18 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On Mon, Feb 25, 2019 at 08:59:36AM -0800, Darrick J. Wong wrote: > On Mon, Feb 25, 2019 at 07:55:02AM -0500, Brian Foster wrote: > > On Sun, Feb 24, 2019 at 12:30:58PM -0800, Darrick J. Wong wrote: > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > Don't pass raw iomap flags to xfs_reflink_allocate_cow; signal our > > > intention with a boolean argument. > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > fs/xfs/xfs_iomap.c | 6 +++--- > > > fs/xfs/xfs_reflink.c | 4 ++-- > > > fs/xfs/xfs_reflink.h | 2 +- > > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > > > > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > > > index 01210eae8bf3..a376fb2d4b94 100644 > > > --- a/fs/xfs/xfs_iomap.c > > > +++ b/fs/xfs/xfs_iomap.c > > > @@ -983,6 +983,7 @@ xfs_file_iomap_begin( > > > */ > > > if (xfs_is_cow_inode(ip)) { > > > struct xfs_bmbt_irec orig = imap; > > > + bool convert_now = (flags & IOMAP_DIRECT); > > > > > > /* if zeroing doesn't need COW allocation, then we are done. */ > > > if ((flags & IOMAP_ZERO) && > > > @@ -991,7 +992,7 @@ xfs_file_iomap_begin( > > > > > > /* may drop and re-acquire the ilock */ > > > error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, > > > - flags); > > > + convert_now); > > > if (error) > > > goto out_unlock; > > > > > > @@ -1002,8 +1003,7 @@ xfs_file_iomap_begin( > > > * direct I/O code, which must be block aligned we need to > > > * report the newly allocated address. > > > */ > > > - if (!(flags & IOMAP_DIRECT) && > > > - orig.br_startblock != HOLESTARTBLOCK) > > > + if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) > > > > FWIW, I thought the original dio check made sense here (in the iomap > > callback), whereas the flags parameter to the external function seemed > > more of a layering hack. Looks fine, anyways: > > Yeah, I went back and forth about this 'convert_now'. The flag is > trying to convey that we need to return the disk location of where we > want the write to land. > > bool need_real_write_location = (flags & IOMAP_DIRECT); > > But that's a long name. "need_write_map", perhaps? > What I mean to say is that (IMO) the existing IOMAP_DIRECT checks seemed fine as long as we changed the param name. FWIW, if we wanted to factor it out to a bool I'd personally just call it 'direct_io' or 'dio' or something in xfs_file_iomap_begin() (and leave it as 'convert' or whatever in the reflink func), but I'm fine with it as is too. Brian > --D > > > Reviewed-by: Brian Foster <bfoster@redhat.com> > > > > > imap = orig; > > > > > > end_fsb = imap.br_startoff + imap.br_blockcount; > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > > > index e2d9179bd50d..d42e3ef9050e 100644 > > > --- a/fs/xfs/xfs_reflink.c > > > +++ b/fs/xfs/xfs_reflink.c > > > @@ -361,7 +361,7 @@ xfs_reflink_allocate_cow( > > > struct xfs_bmbt_irec *imap, > > > bool *shared, > > > uint *lockmode, > > > - unsigned iomap_flags) > > > + bool convert_now) > > > { > > > struct xfs_mount *mp = ip->i_mount; > > > xfs_fileoff_t offset_fsb = imap->br_startoff; > > > @@ -444,7 +444,7 @@ xfs_reflink_allocate_cow( > > > * to initiate a disk write. For direct I/O we are going to write the > > > * data and need the conversion, but for buffered writes we're done. > > > */ > > > - if (!(iomap_flags & IOMAP_DIRECT) || imap->br_state == XFS_EXT_NORM) > > > + if (!convert_now || imap->br_state == XFS_EXT_NORM) > > > return 0; > > > trace_xfs_reflink_convert_cow(ip, imap); > > > return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb); > > > diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h > > > index 2a3052fbe23e..28a43b7f581d 100644 > > > --- a/fs/xfs/xfs_reflink.h > > > +++ b/fs/xfs/xfs_reflink.h > > > @@ -27,7 +27,7 @@ bool xfs_inode_need_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, > > > > > > extern int xfs_reflink_allocate_cow(struct xfs_inode *ip, > > > struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, > > > - unsigned iomap_flags); > > > + bool convert_now); > > > extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset, > > > xfs_off_t count); > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow 2019-02-25 17:18 ` Brian Foster @ 2019-02-25 17:43 ` Darrick J. Wong 0 siblings, 0 replies; 7+ messages in thread From: Darrick J. Wong @ 2019-02-25 17:43 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs On Mon, Feb 25, 2019 at 12:18:54PM -0500, Brian Foster wrote: > On Mon, Feb 25, 2019 at 08:59:36AM -0800, Darrick J. Wong wrote: > > On Mon, Feb 25, 2019 at 07:55:02AM -0500, Brian Foster wrote: > > > On Sun, Feb 24, 2019 at 12:30:58PM -0800, Darrick J. Wong wrote: > > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > > > Don't pass raw iomap flags to xfs_reflink_allocate_cow; signal our > > > > intention with a boolean argument. > > > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > > --- > > > > fs/xfs/xfs_iomap.c | 6 +++--- > > > > fs/xfs/xfs_reflink.c | 4 ++-- > > > > fs/xfs/xfs_reflink.h | 2 +- > > > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > > > > index 01210eae8bf3..a376fb2d4b94 100644 > > > > --- a/fs/xfs/xfs_iomap.c > > > > +++ b/fs/xfs/xfs_iomap.c > > > > @@ -983,6 +983,7 @@ xfs_file_iomap_begin( > > > > */ > > > > if (xfs_is_cow_inode(ip)) { > > > > struct xfs_bmbt_irec orig = imap; > > > > + bool convert_now = (flags & IOMAP_DIRECT); > > > > > > > > /* if zeroing doesn't need COW allocation, then we are done. */ > > > > if ((flags & IOMAP_ZERO) && > > > > @@ -991,7 +992,7 @@ xfs_file_iomap_begin( > > > > > > > > /* may drop and re-acquire the ilock */ > > > > error = xfs_reflink_allocate_cow(ip, &imap, &shared, &lockmode, > > > > - flags); > > > > + convert_now); > > > > if (error) > > > > goto out_unlock; > > > > > > > > @@ -1002,8 +1003,7 @@ xfs_file_iomap_begin( > > > > * direct I/O code, which must be block aligned we need to > > > > * report the newly allocated address. > > > > */ > > > > - if (!(flags & IOMAP_DIRECT) && > > > > - orig.br_startblock != HOLESTARTBLOCK) > > > > + if (!convert_now && orig.br_startblock != HOLESTARTBLOCK) > > > > > > FWIW, I thought the original dio check made sense here (in the iomap > > > callback), whereas the flags parameter to the external function seemed > > > more of a layering hack. Looks fine, anyways: > > > > Yeah, I went back and forth about this 'convert_now'. The flag is > > trying to convey that we need to return the disk location of where we > > want the write to land. > > > > bool need_real_write_location = (flags & IOMAP_DIRECT); > > > > But that's a long name. "need_write_map", perhaps? > > > > What I mean to say is that (IMO) the existing IOMAP_DIRECT checks seemed > fine as long as we changed the param name. FWIW, if we wanted to factor > it out to a bool I'd personally just call it 'direct_io' or 'dio' or > something in xfs_file_iomap_begin() (and leave it as 'convert' or > whatever in the reflink func), but I'm fine with it as is too. <nod> I'll change 'convert_now' -> 'directio' in this function. The reflink function will retain the 'convert_now' name. --D > Brian > > > --D > > > > > Reviewed-by: Brian Foster <bfoster@redhat.com> > > > > > > > imap = orig; > > > > > > > > end_fsb = imap.br_startoff + imap.br_blockcount; > > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > > > > index e2d9179bd50d..d42e3ef9050e 100644 > > > > --- a/fs/xfs/xfs_reflink.c > > > > +++ b/fs/xfs/xfs_reflink.c > > > > @@ -361,7 +361,7 @@ xfs_reflink_allocate_cow( > > > > struct xfs_bmbt_irec *imap, > > > > bool *shared, > > > > uint *lockmode, > > > > - unsigned iomap_flags) > > > > + bool convert_now) > > > > { > > > > struct xfs_mount *mp = ip->i_mount; > > > > xfs_fileoff_t offset_fsb = imap->br_startoff; > > > > @@ -444,7 +444,7 @@ xfs_reflink_allocate_cow( > > > > * to initiate a disk write. For direct I/O we are going to write the > > > > * data and need the conversion, but for buffered writes we're done. > > > > */ > > > > - if (!(iomap_flags & IOMAP_DIRECT) || imap->br_state == XFS_EXT_NORM) > > > > + if (!convert_now || imap->br_state == XFS_EXT_NORM) > > > > return 0; > > > > trace_xfs_reflink_convert_cow(ip, imap); > > > > return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb); > > > > diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h > > > > index 2a3052fbe23e..28a43b7f581d 100644 > > > > --- a/fs/xfs/xfs_reflink.h > > > > +++ b/fs/xfs/xfs_reflink.h > > > > @@ -27,7 +27,7 @@ bool xfs_inode_need_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap, > > > > > > > > extern int xfs_reflink_allocate_cow(struct xfs_inode *ip, > > > > struct xfs_bmbt_irec *imap, bool *shared, uint *lockmode, > > > > - unsigned iomap_flags); > > > > + bool convert_now); > > > > extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset, > > > > xfs_off_t count); > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-25 17:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-24 20:30 [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Darrick J. Wong 2019-02-24 20:31 ` [PATCH 2/2] xfs: rework breaking of shared extents in xfs_file_iomap_begin Darrick J. Wong 2019-02-25 12:55 ` Brian Foster 2019-02-25 12:55 ` [PATCH 1/2] xfs: don't pass iomap flags to xfs_reflink_allocate_cow Brian Foster 2019-02-25 16:59 ` Darrick J. Wong 2019-02-25 17:18 ` Brian Foster 2019-02-25 17:43 ` Darrick J. Wong
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.