On Tue, Jan 23, 2018 at 04:42:24PM +0800, Shan Hai wrote:
> Remove the extent size hint and realtime inode relevant code from
> the xfs_bmapi_reserve_delalloc since it is not called on the inode
> with extent size hint set or on a realtime inode.
>
> Signed-off-by: Shan Hai <shan.hai@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 32 ++++++++------------------------
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1bddbba6b80c..8b4ab1932b58 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3876,8 +3876,6 @@ xfs_bmapi_reserve_delalloc(
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> xfs_extlen_t alen;
> xfs_extlen_t indlen;
> - char rt = XFS_IS_REALTIME_INODE(ip);
> - xfs_extlen_t extsz;
> int error;
> xfs_fileoff_t aoff = off;
>
> @@ -3892,31 +3890,25 @@ xfs_bmapi_reserve_delalloc(
> prealloc = alen - len;
>
> /* Figure out the extent size, adjust alen */
> - if (whichfork == XFS_COW_FORK)
> - extsz = xfs_get_cowextsz_hint(ip);
> - else
> - extsz = xfs_get_extsz_hint(ip);
This patch reminded me that I'd been wondering since the early days of
reflink development -- why /does/ this function have the ability to
align delalloc reservations, considering that none of the write paths
seem to use it? Was this just some nugget from the old days? Will the
CoW fork be ok if its tries to cowextsize align its da reservations?
So I went digging into the git history and came up with aff3a9edb70
wherein Dave discovered that the result of these larger da reservations
was that the whole extent could get allocated, but left some kind of
stale data exposure problem. (I guess because we shoved the whole real
extent into the extent map? I wasn't around in the 3.4 days...)
Therefore, the easiest solution was to go straight to making an
unwritten allocation, and any subsequent read would always return
zeroes, the same as directio does.
The CoW fork doesn't have this problem because even if we create large
da reservations and then allocate the whole extent including parts that
aren't mapped by the page cache, the CoW fork extents aren't used for
reads and they aren't mapped into the (permanent) data fork unless we
actually write something to that range.
> - if (extsz) {
> + if (whichfork == XFS_COW_FORK) {
> struct xfs_bmbt_irec prev;
> + xfs_extlen_t extsz = xfs_get_cowextsz_hint(ip);
Please line up the variable names:
struct xfs_bmbt_irec prev;
xfs_extlen_t extsz = xfs_get_cowextsz_hint(ip);
Otherwise it looks ok. I think I'll just edit this on its way in.
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
>
> if (!xfs_iext_peek_prev_extent(ifp, icur, &prev))
> prev.br_startoff = NULLFILEOFF;
>
> - error = xfs_bmap_extsize_align(mp, got, &prev, extsz, rt, eof,
> + error = xfs_bmap_extsize_align(mp, got, &prev, extsz, 0, eof,
> 1, 0, &aoff, &alen);
> ASSERT(!error);
> }
>
> - if (rt)
> - extsz = alen / mp->m_sb.sb_rextsize;
> -
> /*
> * Make a transaction-less quota reservation for delayed allocation
> * blocks. This number gets adjusted later. We return if we haven't
> * allocated blocks already inside this loop.
> */
> error = xfs_trans_reserve_quota_nblks(NULL, ip, (long)alen, 0,
> - rt ? XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS);
> + XFS_QMOPT_RES_REGBLKS);
> if (error)
> return error;
>
> @@ -3927,12 +3919,7 @@ xfs_bmapi_reserve_delalloc(
> indlen = (xfs_extlen_t)xfs_bmap_worst_indlen(ip, alen);
> ASSERT(indlen > 0);
>
> - if (rt) {
> - error = xfs_mod_frextents(mp, -((int64_t)extsz));
> - } else {
> - error = xfs_mod_fdblocks(mp, -((int64_t)alen), false);
> - }
> -
> + error = xfs_mod_fdblocks(mp, -((int64_t)alen), false);
> if (error)
> goto out_unreserve_quota;
>
> @@ -3963,14 +3950,11 @@ xfs_bmapi_reserve_delalloc(
> return 0;
>
> out_unreserve_blocks:
> - if (rt)
> - xfs_mod_frextents(mp, extsz);
> - else
> - xfs_mod_fdblocks(mp, alen, false);
> + xfs_mod_fdblocks(mp, alen, false);
> out_unreserve_quota:
> if (XFS_IS_QUOTA_ON(mp))
> - xfs_trans_unreserve_quota_nblks(NULL, ip, (long)alen, 0, rt ?
> - XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS);
> + xfs_trans_unreserve_quota_nblks(NULL, ip, (long)alen, 0,
> + XFS_QMOPT_RES_REGBLKS);
> return error;
> }
>
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
