* [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0
@ 2017-09-19 18:10 Darrick J. Wong
2017-09-20 13:23 ` Brian Foster
2017-09-21 14:04 ` Christoph Hellwig
0 siblings, 2 replies; 4+ messages in thread
From: Darrick J. Wong @ 2017-09-19 18:10 UTC (permalink / raw)
To: xfs
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.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_ag_resv.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
index b008ff3..df3e600 100644
--- a/fs/xfs/libxfs/xfs_ag_resv.c
+++ b/fs/xfs/libxfs/xfs_ag_resv.c
@@ -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;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0
2017-09-19 18:10 [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0 Darrick J. Wong
@ 2017-09-20 13:23 ` Brian Foster
2017-09-21 14:04 ` Christoph Hellwig
1 sibling, 0 replies; 4+ messages in thread
From: Brian Foster @ 2017-09-20 13:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Tue, Sep 19, 2017 at 11:10:41AM -0700, Darrick J. Wong wrote:
> 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.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_ag_resv.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> index b008ff3..df3e600 100644
> --- a/fs/xfs/libxfs/xfs_ag_resv.c
> +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0
2017-09-19 18:10 [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0 Darrick J. Wong
2017-09-20 13:23 ` Brian Foster
@ 2017-09-21 14:04 ` Christoph Hellwig
2017-09-21 17:12 ` Darrick J. Wong
1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2017-09-21 14:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Tue, Sep 19, 2017 at 11:10:41AM -0700, Darrick J. Wong wrote:
> 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.
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0
2017-09-21 14:04 ` Christoph Hellwig
@ 2017-09-21 17:12 ` Darrick J. Wong
0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2017-09-21 17:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Thu, Sep 21, 2017 at 07:04:21AM -0700, Christoph Hellwig wrote:
> On Tue, Sep 19, 2017 at 11:10:41AM -0700, Darrick J. Wong wrote:
> > 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.
>
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-21 17:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 18:10 [PATCH] xfs: perag initialization should only touch m_ag_max_usable for AG 0 Darrick J. Wong
2017-09-20 13:23 ` Brian Foster
2017-09-21 14:04 ` Christoph Hellwig
2017-09-21 17:12 ` Darrick J. Wong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.