On Mon, Aug 17, 2020 at 07:53:07AM +0100, Christoph Hellwig wrote:
> On Fri, Aug 14, 2020 at 01:38:25PM +0530, Chandan Babu R wrote:
> > When adding a new data extent (without modifying an inode's existing
> > extents) the extent count increases only by 1. This commit checks for
> > extent count overflow in such cases.
> >
> > Signed-off-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_bmap.c | 8 ++++++++
> > fs/xfs/libxfs/xfs_inode_fork.h | 2 ++
> > fs/xfs/xfs_bmap_util.c | 5 +++++
> > fs/xfs/xfs_dquot.c | 8 +++++++-
> > fs/xfs/xfs_iomap.c | 5 +++++
> > fs/xfs/xfs_rtalloc.c | 5 +++++
> > 6 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index 9c40d5971035..e64f645415b1 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -4527,6 +4527,14 @@ xfs_bmapi_convert_delalloc(
> > return error;
> >
> > xfs_ilock(ip, XFS_ILOCK_EXCL);
> > +
> > + if (whichfork == XFS_DATA_FORK) {
>
> Should we add COW fork special casing to xfs_iext_count_may_overflow
> instead?
>
> > + error = xfs_iext_count_may_overflow(ip, whichfork,
> > + XFS_IEXT_ADD_CNT);
>
> I find the XFS_IEXT_ADD_CNT define very confusing. An explicit 1 passed
> for a counter parameter makes a lot more sense to me.
I explicitly asked Chandan to convert all the magic numbers
sprinkled in the previous patch to defined values. It was impossible
to know whether the intended value was correct when it's just an
open coded number because we don't know what the number actually
stands for. And, in future, if we change the behaviour of a specific
operation, then we only have to change a single value rather than
having to track down and determine if every magic "1" is for an
extent add operation or something different.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
