| Message ID | 20200918094759.2727564-10-chandanrlinux@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bail out if transaction can cause extent count to overflow | expand |
On Fri, Sep 18, 2020 at 03:17:58PM +0530, Chandan Babu R wrote: > Removing an initial range of source/donor file's extent and adding a new > extent (from donor/source file) in its place will cause extent count to > increase by 1. > > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> > --- > fs/xfs/libxfs/xfs_bmap.c | 18 +++++++++--------- > fs/xfs/libxfs/xfs_bmap.h | 1 + > fs/xfs/libxfs/xfs_inode_fork.h | 7 +++++++ > fs/xfs/xfs_bmap_util.c | 17 +++++++++++++++++ > 4 files changed, 34 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 51c2d2690f05..9c665e379dfc 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -6104,15 +6104,6 @@ xfs_bmap_split_extent( > return error; > } > > -/* Deferred mapping is only for real extents in the data fork. */ > -static bool > -xfs_bmap_is_update_needed( > - struct xfs_bmbt_irec *bmap) > -{ > - return bmap->br_startblock != HOLESTARTBLOCK && > - bmap->br_startblock != DELAYSTARTBLOCK; > -} > - > /* Record a bmap intent. */ > static int > __xfs_bmap_add( > @@ -6144,6 +6135,15 @@ __xfs_bmap_add( > return 0; > } > > +/* Deferred mapping is only for real extents in the data fork. */ > +bool > +xfs_bmap_is_update_needed( > + struct xfs_bmbt_irec *bmap) > +{ > + return bmap->br_startblock != HOLESTARTBLOCK && > + bmap->br_startblock != DELAYSTARTBLOCK; > +} I think the predicate you want below is xfs_bmap_is_real_extent(). (I think that mostly because I'm going to kill this predicate entirely in a patch for the next cycle, because it is redundant and _is_real_extent is a better name.) --D > + > /* Map an extent into a file. */ > void > xfs_bmap_map_extent( > diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h > index e1bd484e5548..60fbe184d5f4 100644 > --- a/fs/xfs/libxfs/xfs_bmap.h > +++ b/fs/xfs/libxfs/xfs_bmap.h > @@ -263,6 +263,7 @@ struct xfs_bmap_intent { > struct xfs_bmbt_irec bi_bmap; > }; > > +bool xfs_bmap_is_update_needed(struct xfs_bmbt_irec *bmap); > int xfs_bmap_finish_one(struct xfs_trans *tp, struct xfs_inode *ip, > enum xfs_bmap_intent_type type, int whichfork, > xfs_fileoff_t startoff, xfs_fsblock_t startblock, > diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h > index ded3c1b56c94..837c01595439 100644 > --- a/fs/xfs/libxfs/xfs_inode_fork.h > +++ b/fs/xfs/libxfs/xfs_inode_fork.h > @@ -102,6 +102,13 @@ struct xfs_ifork { > #define XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written) \ > (((smap_real) ? 1 : 0) + ((dmap_written) ? 1 : 0)) > > +/* > + * Removing an initial range of source/donor file's extent and adding a new > + * extent (from donor/source file) in its place will cause extent count to > + * increase by 1. > + */ > +#define XFS_IEXT_SWAP_RMAP_CNT (1) > + > /* > * Fork handling. > */ > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index 0776abd0103c..542f990247c4 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -28,6 +28,7 @@ > #include "xfs_icache.h" > #include "xfs_iomap.h" > #include "xfs_reflink.h" > +#include "xfs_bmap.h" > > /* Kernel only BMAP related definitions and functions */ > > @@ -1407,6 +1408,22 @@ xfs_swap_extent_rmap( > irec.br_blockcount); > trace_xfs_swap_extent_rmap_remap_piece(tip, &uirec); > > + if (xfs_bmap_is_update_needed(&uirec)) { > + error = xfs_iext_count_may_overflow(ip, > + XFS_DATA_FORK, > + XFS_IEXT_SWAP_RMAP_CNT); > + if (error) > + goto out; > + } > + > + if (xfs_bmap_is_update_needed(&irec)) { > + error = xfs_iext_count_may_overflow(tip, > + XFS_DATA_FORK, > + XFS_IEXT_SWAP_RMAP_CNT); > + if (error) > + goto out; > + } > + > /* Remove the mapping from the donor file. */ > xfs_bmap_unmap_extent(tp, tip, &uirec); > > -- > 2.28.0 >
On Friday 18 September 2020 9:14:45 PM IST Darrick J. Wong wrote: > On Fri, Sep 18, 2020 at 03:17:58PM +0530, Chandan Babu R wrote: > > Removing an initial range of source/donor file's extent and adding a new > > extent (from donor/source file) in its place will cause extent count to > > increase by 1. > > > > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> > > --- > > fs/xfs/libxfs/xfs_bmap.c | 18 +++++++++--------- > > fs/xfs/libxfs/xfs_bmap.h | 1 + > > fs/xfs/libxfs/xfs_inode_fork.h | 7 +++++++ > > fs/xfs/xfs_bmap_util.c | 17 +++++++++++++++++ > > 4 files changed, 34 insertions(+), 9 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > index 51c2d2690f05..9c665e379dfc 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.c > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > @@ -6104,15 +6104,6 @@ xfs_bmap_split_extent( > > return error; > > } > > > > -/* Deferred mapping is only for real extents in the data fork. */ > > -static bool > > -xfs_bmap_is_update_needed( > > - struct xfs_bmbt_irec *bmap) > > -{ > > - return bmap->br_startblock != HOLESTARTBLOCK && > > - bmap->br_startblock != DELAYSTARTBLOCK; > > -} > > - > > /* Record a bmap intent. */ > > static int > > __xfs_bmap_add( > > @@ -6144,6 +6135,15 @@ __xfs_bmap_add( > > return 0; > > } > > > > +/* Deferred mapping is only for real extents in the data fork. */ > > +bool > > +xfs_bmap_is_update_needed( > > + struct xfs_bmbt_irec *bmap) > > +{ > > + return bmap->br_startblock != HOLESTARTBLOCK && > > + bmap->br_startblock != DELAYSTARTBLOCK; > > +} > > I think the predicate you want below is xfs_bmap_is_real_extent(). Yes, that is indeed correct. I will fix this one too. > > (I think that mostly because I'm going to kill this predicate entirely > in a patch for the next cycle, because it is redundant and > _is_real_extent is a better name.) > > --D > > > + > > /* Map an extent into a file. */ > > void > > xfs_bmap_map_extent( > > diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h > > index e1bd484e5548..60fbe184d5f4 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.h > > +++ b/fs/xfs/libxfs/xfs_bmap.h > > @@ -263,6 +263,7 @@ struct xfs_bmap_intent { > > struct xfs_bmbt_irec bi_bmap; > > }; > > > > +bool xfs_bmap_is_update_needed(struct xfs_bmbt_irec *bmap); > > int xfs_bmap_finish_one(struct xfs_trans *tp, struct xfs_inode *ip, > > enum xfs_bmap_intent_type type, int whichfork, > > xfs_fileoff_t startoff, xfs_fsblock_t startblock, > > diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h > > index ded3c1b56c94..837c01595439 100644 > > --- a/fs/xfs/libxfs/xfs_inode_fork.h > > +++ b/fs/xfs/libxfs/xfs_inode_fork.h > > @@ -102,6 +102,13 @@ struct xfs_ifork { > > #define XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written) \ > > (((smap_real) ? 1 : 0) + ((dmap_written) ? 1 : 0)) > > > > +/* > > + * Removing an initial range of source/donor file's extent and adding a new > > + * extent (from donor/source file) in its place will cause extent count to > > + * increase by 1. > > + */ > > +#define XFS_IEXT_SWAP_RMAP_CNT (1) > > + > > /* > > * Fork handling. > > */ > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > > index 0776abd0103c..542f990247c4 100644 > > --- a/fs/xfs/xfs_bmap_util.c > > +++ b/fs/xfs/xfs_bmap_util.c > > @@ -28,6 +28,7 @@ > > #include "xfs_icache.h" > > #include "xfs_iomap.h" > > #include "xfs_reflink.h" > > +#include "xfs_bmap.h" > > > > /* Kernel only BMAP related definitions and functions */ > > > > @@ -1407,6 +1408,22 @@ xfs_swap_extent_rmap( > > irec.br_blockcount); > > trace_xfs_swap_extent_rmap_remap_piece(tip, &uirec); > > > > + if (xfs_bmap_is_update_needed(&uirec)) { > > + error = xfs_iext_count_may_overflow(ip, > > + XFS_DATA_FORK, > > + XFS_IEXT_SWAP_RMAP_CNT); > > + if (error) > > + goto out; > > + } > > + > > + if (xfs_bmap_is_update_needed(&irec)) { > > + error = xfs_iext_count_may_overflow(tip, > > + XFS_DATA_FORK, > > + XFS_IEXT_SWAP_RMAP_CNT); > > + if (error) > > + goto out; > > + } > > + > > /* Remove the mapping from the donor file. */ > > xfs_bmap_unmap_extent(tp, tip, &uirec); > > >
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 51c2d2690f05..9c665e379dfc 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -6104,15 +6104,6 @@ xfs_bmap_split_extent( return error; } -/* Deferred mapping is only for real extents in the data fork. */ -static bool -xfs_bmap_is_update_needed( - struct xfs_bmbt_irec *bmap) -{ - return bmap->br_startblock != HOLESTARTBLOCK && - bmap->br_startblock != DELAYSTARTBLOCK; -} - /* Record a bmap intent. */ static int __xfs_bmap_add( @@ -6144,6 +6135,15 @@ __xfs_bmap_add( return 0; } +/* Deferred mapping is only for real extents in the data fork. */ +bool +xfs_bmap_is_update_needed( + struct xfs_bmbt_irec *bmap) +{ + return bmap->br_startblock != HOLESTARTBLOCK && + bmap->br_startblock != DELAYSTARTBLOCK; +} + /* Map an extent into a file. */ void xfs_bmap_map_extent( diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h index e1bd484e5548..60fbe184d5f4 100644 --- a/fs/xfs/libxfs/xfs_bmap.h +++ b/fs/xfs/libxfs/xfs_bmap.h @@ -263,6 +263,7 @@ struct xfs_bmap_intent { struct xfs_bmbt_irec bi_bmap; }; +bool xfs_bmap_is_update_needed(struct xfs_bmbt_irec *bmap); int xfs_bmap_finish_one(struct xfs_trans *tp, struct xfs_inode *ip, enum xfs_bmap_intent_type type, int whichfork, xfs_fileoff_t startoff, xfs_fsblock_t startblock, diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h index ded3c1b56c94..837c01595439 100644 --- a/fs/xfs/libxfs/xfs_inode_fork.h +++ b/fs/xfs/libxfs/xfs_inode_fork.h @@ -102,6 +102,13 @@ struct xfs_ifork { #define XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written) \ (((smap_real) ? 1 : 0) + ((dmap_written) ? 1 : 0)) +/* + * Removing an initial range of source/donor file's extent and adding a new + * extent (from donor/source file) in its place will cause extent count to + * increase by 1. + */ +#define XFS_IEXT_SWAP_RMAP_CNT (1) + /* * Fork handling. */ diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 0776abd0103c..542f990247c4 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -28,6 +28,7 @@ #include "xfs_icache.h" #include "xfs_iomap.h" #include "xfs_reflink.h" +#include "xfs_bmap.h" /* Kernel only BMAP related definitions and functions */ @@ -1407,6 +1408,22 @@ xfs_swap_extent_rmap( irec.br_blockcount); trace_xfs_swap_extent_rmap_remap_piece(tip, &uirec); + if (xfs_bmap_is_update_needed(&uirec)) { + error = xfs_iext_count_may_overflow(ip, + XFS_DATA_FORK, + XFS_IEXT_SWAP_RMAP_CNT); + if (error) + goto out; + } + + if (xfs_bmap_is_update_needed(&irec)) { + error = xfs_iext_count_may_overflow(tip, + XFS_DATA_FORK, + XFS_IEXT_SWAP_RMAP_CNT); + if (error) + goto out; + } + /* Remove the mapping from the donor file. */ xfs_bmap_unmap_extent(tp, tip, &uirec);
Removing an initial range of source/donor file's extent and adding a new extent (from donor/source file) in its place will cause extent count to increase by 1. Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> --- fs/xfs/libxfs/xfs_bmap.c | 18 +++++++++--------- fs/xfs/libxfs/xfs_bmap.h | 1 + fs/xfs/libxfs/xfs_inode_fork.h | 7 +++++++ fs/xfs/xfs_bmap_util.c | 17 +++++++++++++++++ 4 files changed, 34 insertions(+), 9 deletions(-)