On 11 Aug 2021 at 17:58, "Darrick J. Wong" <djwong@xxxxxxxxxx> wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> In commit 8ad560d2565e, we changed xfs_rtalloc_query_range to constrain
> the range of bits in the realtime bitmap file that would actually be
> searched. In commit a3a374bf1889, we changed the range again
> (incorrectly), leading to the fix in commit d88850bd5516, which finally
> corrected the range check code. Unfortunately, the author never noticed
> that the function modifies its input parameters, which is a totaly no-no
> since none of the other range query functions change their input
> parameters.
>
> So, fix this function yet again to stash the upper end of the query
> range (i.e. the high key) in a local variable and hope this is the last
> time I have to fix my own function. While we're at it, mark the key
> inputs const so nobody makes this mistake again. :(
>
Looks good.
Reviewed-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
> Fixes: 8ad560d2565e ("xfs: strengthen rtalloc query range checks")
> Not-fixed-by: a3a374bf1889 ("xfs: fix off-by-one error in xfs_rtalloc_query_range")
> Not-fixed-by: d88850bd5516 ("xfs: fix high key handling in the rt allocator's query_range function")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_rtbitmap.c | 14 +++++++-------
> fs/xfs/xfs_rtalloc.h | 7 +++----
> 2 files changed, 10 insertions(+), 11 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index 483375c6a735..5740ba664867 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1009,8 +1009,8 @@ xfs_rtfree_extent(
> int
> xfs_rtalloc_query_range(
> struct xfs_trans *tp,
> - struct xfs_rtalloc_rec *low_rec,
> - struct xfs_rtalloc_rec *high_rec,
> + const struct xfs_rtalloc_rec *low_rec,
> + const struct xfs_rtalloc_rec *high_rec,
> xfs_rtalloc_query_range_fn fn,
> void *priv)
> {
> @@ -1018,6 +1018,7 @@ xfs_rtalloc_query_range(
> struct xfs_mount *mp = tp->t_mountp;
> xfs_rtblock_t rtstart;
> xfs_rtblock_t rtend;
> + xfs_rtblock_t high_key;
> int is_free;
> int error = 0;
>
> @@ -1026,12 +1027,12 @@ xfs_rtalloc_query_range(
> if (low_rec->ar_startext >= mp->m_sb.sb_rextents ||
> low_rec->ar_startext == high_rec->ar_startext)
> return 0;
> - high_rec->ar_startext = min(high_rec->ar_startext,
> - mp->m_sb.sb_rextents - 1);
> +
> + high_key = min(high_rec->ar_startext, mp->m_sb.sb_rextents - 1);
>
> /* Iterate the bitmap, looking for discrepancies. */
> rtstart = low_rec->ar_startext;
> - while (rtstart <= high_rec->ar_startext) {
> + while (rtstart <= high_key) {
> /* Is the first block free? */
> error = xfs_rtcheck_range(mp, tp, rtstart, 1, 1, &rtend,
> &is_free);
> @@ -1039,8 +1040,7 @@ xfs_rtalloc_query_range(
> break;
>
> /* How long does the extent go for? */
> - error = xfs_rtfind_forw(mp, tp, rtstart,
> - high_rec->ar_startext, &rtend);
> + error = xfs_rtfind_forw(mp, tp, rtstart, high_key, &rtend);
> if (error)
> break;
>
> diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h
> index ed885620589c..51097cb24311 100644
> --- a/fs/xfs/xfs_rtalloc.h
> +++ b/fs/xfs/xfs_rtalloc.h
> @@ -124,10 +124,9 @@ int xfs_rtfree_range(struct xfs_mount *mp, struct xfs_trans *tp,
> xfs_rtblock_t start, xfs_extlen_t len,
> struct xfs_buf **rbpp, xfs_fsblock_t *rsb);
> int xfs_rtalloc_query_range(struct xfs_trans *tp,
> - struct xfs_rtalloc_rec *low_rec,
> - struct xfs_rtalloc_rec *high_rec,
> - xfs_rtalloc_query_range_fn fn,
> - void *priv);
> + const struct xfs_rtalloc_rec *low_rec,
> + const struct xfs_rtalloc_rec *high_rec,
> + xfs_rtalloc_query_range_fn fn, void *priv);
> int xfs_rtalloc_query_all(struct xfs_trans *tp,
> xfs_rtalloc_query_range_fn fn,
> void *priv);
--
chandan
