xfs: perag initialization should only touch m_ag_max_usable for AG 0
PATCH 1/1 xfs: perag initialization should only touch m_ag_max_usable for AG 0
We call __xfs_ag_resv_init to make a per-AG reservation for each AG. This makes the reservation per-AG, not per-filesystem. Therefore, it is incorrect to adjust m_ag_max_usable for each AG. Adjust it only when we're reserving AG 0's blocks so that we only do it once per fs.
--- fs/xfs/libxfs/xfs_ag_resv.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
fs/xfs/libxfs/xfs_ag_resv.c +10 -2
@@ -156,7 +156,8 @@ __xfs_ag_resv_free(trace_xfs_ag_resv_free (pag, type,0 ); resv =xfs_perag_resv (pag, type); - pag->pag_mount->m_ag_max_usable += resv->ar_asked; +if (pag->pag_agno ==0 ) + pag->pag_mount->m_ag_max_usable += resv->ar_asked;/* * AGFL blocks are always considered "free", so whatever * was reserved at mount time must be given back at umount.
@@ -216,7 +217,14 @@ __xfs_ag_resv_init(return error; } - mp->m_ag_max_usable -= ask; +/* +* Reduce the maximum per-AG allocation length by however much we're +* trying to reserve for an AG. Since this is a filesystem-wide +* counter, we only make the adjustment for AG 0. This assumes that +* there aren't any AGs hungrier for per-AG reservation than AG 0. +*/ +if (pag->pag_agno ==0 ) + mp->m_ag_max_usable -= ask; resv =xfs_perag_resv (pag, type); resv->ar_asked = ask;
Quoted 7 lines · Darrick J. Wong
We call
__xfs_ag_resv_initto make a per-AG reservation for each AG. This makes the reservation per-AG, not per-filesystem. Therefore, it is incorrect to adjustm_ag_max_usablefor each AG. Adjust it only when we're reserving AG 0's blocks so that we only do it once per fs.Signed-off-by Darrick J. Wong <[email protected]>---
Quoted 37 lines
fs/xfs/libxfs/xfs_ag_resv.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)fs/xfs/libxfs/xfs_ag_resv.c +10
-3@@ -156,7 +156,8 @@ __xfs_ag_resv_free(trace_xfs_ag_resv_free (pag, type,0 ); resv =xfs_perag_resv (pag, type); - pag->pag_mount->m_ag_max_usable += resv->ar_asked; +if (pag->pag_agno ==0 ) + pag->pag_mount->m_ag_max_usable += resv->ar_asked;/* * AGFL blocks are always considered "free", so whatever * was reserved at mount time must be given back at umount. @@ -216,7 +217,14 @@ __xfs_ag_resv_init(return error; } - mp->m_ag_max_usable -= ask; +/* +* Reduce the maximum per-AG allocation length by however much we're +* trying to reserve for an AG. Since this is a filesystem-wide +* counter, we only make the adjustment for AG 0. This assumes that +* there aren't any AGs hungrier for per-AG reservation than AG 0. +*/ +if (pag->pag_agno ==0 ) + mp->m_ag_max_usable -= ask; resv =xfs_perag_resv (pag, type); resv->ar_asked = ask; --To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 4 lines · Darrick J. Wong
We call
__xfs_ag_resv_initto make a per-AG reservation for each AG. This makes the reservation per-AG, not per-filesystem. Therefore, it is incorrect to adjustm_ag_max_usablefor each AG. Adjust it only when we're reserving AG 0's blocks so that we only do it once per fs.
This looks correct to me, but I hate how we manipulate a per-mount value. Is there a way we can just move this to the caller?
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Darrick J. Wong
We call
__xfs_ag_resv_initto make a per-AG reservation for each AG. This makes the reservation per-AG, not per-filesystem. Therefore, it is incorrect to adjustm_ag_max_usablefor each AG. Adjust it only when we're reserving AG 0's blocks so that we only do it once per fs.This looks correct to me, but I hate how we manipulate a per-mount value. Is there a way we can just move this to the caller?
I'm not sure which caller you're referring to -- if you mean xfs_ag_resv_init, then we'd still have the "if (pag->agno == 0) xfs_mod_fdblocks(...)"; and if xfs_fs_reserve_ag_blocks then you'd have to bubble the two ask values up out of xfs_ag_resv_init, which also seems like a layering violation. Modifying fdblocks in __xfs_ag_resv_init (Dave I think is sharpening his axe on that) smells kinda funny but it's less code intensive.
<confused>
--D
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html