On Monday 17 August 2020 12:23:07 PM IST 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?
I agree. Making xfs_iext_count_may_overflow() to always return success in the
case of CoW fork would mean that the if condition can be removed and hence
makes the code more readable.
>
> > + 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.
The reason to do this was to consolidate the comment descriptions at one
place. For e.g. the comment for XFS_IEXT_DIR_MANIP_CNT (from "[PATCH V2 05/10]
xfs: Check for extent overflow when adding/removing dir entries") is slightly
larger. Using constants (instead of macros) would mean that the same comment
has to be replicated across the 6 locations it is being used.
--
chandan
