| Message ID | 20200224040044.30923-3-chandanrlinux@gmail.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | Fix log reservation calculation for xattr insert operation | expand |
On Mon, Feb 24, 2020 at 09:30:39AM +0530, Chandan Rajendra wrote: > The size calculated by xfs_attr_calc_size() is a sum of three components, > 1. Number of dabtree blocks > 2. Number of Bmbt blocks > 3. Number of remote blocks > > This commit introduces new local variables to track these numbers explicitly. > > Signed-off-by: Chandan Rajendra <chandanrlinux@gmail.com> > --- > fs/xfs/libxfs/xfs_attr.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 1875210cc8e40..942ba552e0bdd 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -142,8 +142,10 @@ xfs_attr_calc_size( > int *local) > { > struct xfs_mount *mp = args->dp->i_mount; > + unsigned int total_dablks; > + unsigned int bmbt_blks; > + unsigned int rmt_blks; > int size; > - int nblks; > > /* > * Determine space new attribute will use, and if it would be > @@ -151,23 +153,26 @@ xfs_attr_calc_size( > */ > size = xfs_attr_leaf_newentsize(args->geo, args->namelen, > args->valuelen, local); > - nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK); > + total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK); > + bmbt_blks = XFS_DAENTER_BMAPS(mp, XFS_ATTR_FORK); > if (*local) { > if (size > (args->geo->blksize / 2)) { > /* Double split possible */ > - nblks *= 2; > + total_dablks *= 2; > + bmbt_blks *= 2; > } > + rmt_blks = 0; I'd just initialize this one to zero above. Otherwise looks fine: Reviewed-by: Brian Foster <bfoster@redhat.com> > } else { > /* > * Out of line attribute, cannot double split, but > * make room for the attribute value itself. > */ > - uint dblocks = xfs_attr3_rmt_blocks(mp, args->valuelen); > - nblks += dblocks; > - nblks += XFS_NEXTENTADD_SPACE_RES(mp, dblocks, XFS_ATTR_FORK); > + rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen); > + bmbt_blks += XFS_NEXTENTADD_SPACE_RES(mp, rmt_blks, > + XFS_ATTR_FORK); > } > > - return nblks; > + return total_dablks + rmt_blks + bmbt_blks; > } > > STATIC int > -- > 2.19.1 >
On Tuesday, February 25, 2020 9:41 PM Brian Foster wrote: > On Mon, Feb 24, 2020 at 09:30:39AM +0530, Chandan Rajendra wrote: > > The size calculated by xfs_attr_calc_size() is a sum of three components, > > 1. Number of dabtree blocks > > 2. Number of Bmbt blocks > > 3. Number of remote blocks > > > > This commit introduces new local variables to track these numbers explicitly. > > > > Signed-off-by: Chandan Rajendra <chandanrlinux@gmail.com> > > --- > > fs/xfs/libxfs/xfs_attr.c | 19 ++++++++++++------- > > 1 file changed, 12 insertions(+), 7 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > > index 1875210cc8e40..942ba552e0bdd 100644 > > --- a/fs/xfs/libxfs/xfs_attr.c > > +++ b/fs/xfs/libxfs/xfs_attr.c > > @@ -142,8 +142,10 @@ xfs_attr_calc_size( > > int *local) > > { > > struct xfs_mount *mp = args->dp->i_mount; > > + unsigned int total_dablks; > > + unsigned int bmbt_blks; > > + unsigned int rmt_blks; > > int size; > > - int nblks; > > > > /* > > * Determine space new attribute will use, and if it would be > > @@ -151,23 +153,26 @@ xfs_attr_calc_size( > > */ > > size = xfs_attr_leaf_newentsize(args->geo, args->namelen, > > args->valuelen, local); > > - nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK); > > + total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK); > > + bmbt_blks = XFS_DAENTER_BMAPS(mp, XFS_ATTR_FORK); > > if (*local) { > > if (size > (args->geo->blksize / 2)) { > > /* Double split possible */ > > - nblks *= 2; > > + total_dablks *= 2; > > + bmbt_blks *= 2; > > } > > + rmt_blks = 0; > > I'd just initialize this one to zero above. Otherwise looks fine: > > Reviewed-by: Brian Foster <bfoster@redhat.com> Thanks for the review. I will fix this up in the next iteration of the patchset.
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 1875210cc8e40..942ba552e0bdd 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -142,8 +142,10 @@ xfs_attr_calc_size( int *local) { struct xfs_mount *mp = args->dp->i_mount; + unsigned int total_dablks; + unsigned int bmbt_blks; + unsigned int rmt_blks; int size; - int nblks; /* * Determine space new attribute will use, and if it would be @@ -151,23 +153,26 @@ xfs_attr_calc_size( */ size = xfs_attr_leaf_newentsize(args->geo, args->namelen, args->valuelen, local); - nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK); + total_dablks = XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK); + bmbt_blks = XFS_DAENTER_BMAPS(mp, XFS_ATTR_FORK); if (*local) { if (size > (args->geo->blksize / 2)) { /* Double split possible */ - nblks *= 2; + total_dablks *= 2; + bmbt_blks *= 2; } + rmt_blks = 0; } else { /* * Out of line attribute, cannot double split, but * make room for the attribute value itself. */ - uint dblocks = xfs_attr3_rmt_blocks(mp, args->valuelen); - nblks += dblocks; - nblks += XFS_NEXTENTADD_SPACE_RES(mp, dblocks, XFS_ATTR_FORK); + rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen); + bmbt_blks += XFS_NEXTENTADD_SPACE_RES(mp, rmt_blks, + XFS_ATTR_FORK); } - return nblks; + return total_dablks + rmt_blks + bmbt_blks; } STATIC int
The size calculated by xfs_attr_calc_size() is a sum of three components, 1. Number of dabtree blocks 2. Number of Bmbt blocks 3. Number of remote blocks This commit introduces new local variables to track these numbers explicitly. Signed-off-by: Chandan Rajendra <chandanrlinux@gmail.com> --- fs/xfs/libxfs/xfs_attr.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)