From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org, hch@infradead.org, david@fromorbit.com
Subject: Re: [PATCH 06/13] xfs: reserve data and rt quota at the same time
Date: Thu, 28 Jan 2021 13:10:21 -0500 [thread overview]
Message-ID: <20210128181021.GE2619139@bfoster> (raw)
In-Reply-To: <161181369834.1523592.7003018155732921879.stgit@magnolia>
On Wed, Jan 27, 2021 at 10:01:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Modify xfs_trans_reserve_quota_nblks so that we can reserve data and
> realtime blocks from the dquot at the same time. This change has the
> theoretical side effect that for allocations to realtime files we will
> reserve from the dquot both the number of rtblocks being allocated and
> the number of bmbt blocks that might be needed to add the mapping.
> However, since the mount code disables quota if it finds a realtime
> device, this should not result in any behavior changes.
>
> This also replaces the flags argument with a force? boolean since we
s/?//
> don't need to distinguish between data and rt quota reservations any
> more, and the only other flag being passed in was FORCE_RES.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_attr.c | 6 +-----
> fs/xfs/libxfs/xfs_bmap.c | 4 +---
> fs/xfs/xfs_bmap_util.c | 20 +++++++++-----------
> fs/xfs/xfs_iomap.c | 26 +++++++++++++-------------
> fs/xfs/xfs_quota.h | 10 +++++-----
> fs/xfs/xfs_reflink.c | 6 ++----
> fs/xfs/xfs_trans_dquot.c | 42 ++++++++++++++++++++++++++++--------------
> 7 files changed, 59 insertions(+), 55 deletions(-)
>
>
...
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index f0a8f3377281..d54d9f02d3dd 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
...
> @@ -792,18 +792,17 @@ xfs_alloc_file_space(
> if (unlikely(rt)) {
> resrtextents = qblocks = resblks;
This looks like the last usage of qblocks in the function.
> resrtextents /= mp->m_sb.sb_rextsize;
> - resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> - quota_flag = XFS_QMOPT_RES_RTBLKS;
> + dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> + rblocks = resblks;
> } else {
> - resrtextents = 0;
> - resblks = qblocks = XFS_DIOSTRAT_SPACE_RES(mp, resblks);
> - quota_flag = XFS_QMOPT_RES_REGBLKS;
> + dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resblks);
> + rblocks = 0;
> }
>
> /*
> * Allocate and setup the transaction.
> */
> - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks,
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, dblocks,
> resrtextents, 0, &tp);
>
> /*
...
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index de0e371ba4dd..ef29d44c656a 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
...
> @@ -235,18 +235,19 @@ xfs_iomap_write_direct(
> if (IS_DAX(VFS_I(ip))) {
> bmapi_flags = XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO;
> if (imap->br_state == XFS_EXT_UNWRITTEN) {
> + force = true;
> tflags |= XFS_TRANS_RESERVE;
> - resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0) << 1;
> + dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0) << 1;
I'm a little confused what this hunk of logic is for in the first place,
but doesn't this also adjust down the quota where it previously only
affected the transaction reservation? Is that intentional?
Brian
> }
> }
> - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, resrtextents,
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, dblocks, resrtextents,
> tflags, &tp);
> if (error)
> return error;
>
> xfs_ilock(ip, XFS_ILOCK_EXCL);
>
> - error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0, quota_flag);
> + error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
> if (error)
> goto out_trans_cancel;
>
> @@ -559,8 +560,7 @@ xfs_iomap_write_unwritten(
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> xfs_trans_ijoin(tp, ip, 0);
>
> - error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0,
> - XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_FORCE_RES);
> + error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0, true);
> if (error)
> goto error_on_bmapi_transaction;
>
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index 72f4cfc49048..efd04f84d9b4 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -81,8 +81,8 @@ extern void xfs_trans_mod_dquot_byino(struct xfs_trans *, struct xfs_inode *,
> uint, int64_t);
> extern void xfs_trans_apply_dquot_deltas(struct xfs_trans *);
> extern void xfs_trans_unreserve_and_mod_dquots(struct xfs_trans *);
> -extern int xfs_trans_reserve_quota_nblks(struct xfs_trans *,
> - struct xfs_inode *, int64_t, long, uint);
> +int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp, struct xfs_inode *ip,
> + int64_t dblocks, int64_t rblocks, bool force);
> extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
> struct xfs_mount *, struct xfs_dquot *,
> struct xfs_dquot *, struct xfs_dquot *, int64_t, long, uint);
> @@ -114,8 +114,7 @@ extern void xfs_qm_unmount_quotas(struct xfs_mount *);
> static inline int
> xfs_quota_reserve_blkres(struct xfs_inode *ip, int64_t dblocks)
> {
> - return xfs_trans_reserve_quota_nblks(NULL, ip, dblocks, 0,
> - XFS_QMOPT_RES_REGBLKS);
> + return xfs_trans_reserve_quota_nblks(NULL, ip, dblocks, 0, false);
> }
> #else
> static inline int
> @@ -134,7 +133,8 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip, kuid_t kuid, kgid_t kgid,
> #define xfs_trans_apply_dquot_deltas(tp)
> #define xfs_trans_unreserve_and_mod_dquots(tp)
> static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp,
> - struct xfs_inode *ip, int64_t nblks, long ninos, uint flags)
> + struct xfs_inode *ip, int64_t dblocks, int64_t rblocks,
> + bool force)
> {
> return 0;
> }
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 15435229bc1f..0778b5810c26 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -398,8 +398,7 @@ xfs_reflink_allocate_cow(
> goto convert;
> }
>
> - error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0,
> - XFS_QMOPT_RES_REGBLKS);
> + error = xfs_trans_reserve_quota_nblks(tp, ip, resblks, 0, false);
> if (error)
> goto out_trans_cancel;
>
> @@ -1090,8 +1089,7 @@ xfs_reflink_remap_extent(
> if (!smap_real && dmap_written)
> qres += dmap->br_blockcount;
> if (qres > 0) {
> - error = xfs_trans_reserve_quota_nblks(tp, ip, qres, 0,
> - XFS_QMOPT_RES_REGBLKS);
> + error = xfs_trans_reserve_quota_nblks(tp, ip, qres, 0, false);
> if (error)
> goto out_cancel;
> }
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 22aa875b84f7..a1a72b7900c5 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -780,28 +780,42 @@ int
> xfs_trans_reserve_quota_nblks(
> struct xfs_trans *tp,
> struct xfs_inode *ip,
> - int64_t nblks,
> - long ninos,
> - uint flags)
> + int64_t dblocks,
> + int64_t rblocks,
> + bool force)
> {
> struct xfs_mount *mp = ip->i_mount;
> + unsigned int qflags = 0;
> + int error;
>
> if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
> return 0;
>
> ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
> -
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> - ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
> - (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
> -
> - /*
> - * Reserve nblks against these dquots, with trans as the mediator.
> - */
> - return xfs_trans_reserve_quota_bydquots(tp, mp,
> - ip->i_udquot, ip->i_gdquot,
> - ip->i_pdquot,
> - nblks, ninos, flags);
> +
> + if (force)
> + qflags |= XFS_QMOPT_FORCE_RES;
> +
> + /* Reserve data device quota against the inode's dquots. */
> + error = xfs_trans_reserve_quota_bydquots(tp, mp, ip->i_udquot,
> + ip->i_gdquot, ip->i_pdquot, dblocks, 0,
> + XFS_QMOPT_RES_REGBLKS | qflags);
> + if (error)
> + return error;
> +
> + /* Do the same but for realtime blocks. */
> + error = xfs_trans_reserve_quota_bydquots(tp, mp, ip->i_udquot,
> + ip->i_gdquot, ip->i_pdquot, rblocks, 0,
> + XFS_QMOPT_RES_RTBLKS | qflags);
> + if (error) {
> + xfs_trans_reserve_quota_bydquots(tp, mp, ip->i_udquot,
> + ip->i_gdquot, ip->i_pdquot, -dblocks, 0,
> + XFS_QMOPT_RES_REGBLKS);
> + return error;
> + }
> +
> + return 0;
> }
>
> /* Change the quota reservations for an inode creation activity. */
>
next prev parent reply other threads:[~2021-01-28 18:14 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-28 6:01 [PATCHSET v3 00/13] xfs: minor cleanups of the quota functions Darrick J. Wong
2021-01-28 6:01 ` [PATCH 01/13] xfs: clean up quota reservation callsites Darrick J. Wong
2021-01-28 6:01 ` [PATCH 02/13] xfs: create convenience wrappers for incore quota block reservations Darrick J. Wong
2021-01-28 18:08 ` Brian Foster
2021-01-28 6:01 ` [PATCH 03/13] xfs: remove xfs_trans_unreserve_quota_nblks completely Darrick J. Wong
2021-01-28 9:46 ` Christoph Hellwig
2021-01-28 18:08 ` Brian Foster
2021-01-28 6:01 ` [PATCH 04/13] xfs: clean up icreate quota reservation calls Darrick J. Wong
2021-01-28 18:09 ` Brian Foster
2021-01-28 6:01 ` [PATCH 05/13] xfs: fix up build warnings when quotas are disabled Darrick J. Wong
2021-01-28 18:09 ` Brian Foster
2021-01-28 18:22 ` Darrick J. Wong
2021-01-28 18:23 ` Christoph Hellwig
2021-01-28 6:01 ` [PATCH 06/13] xfs: reserve data and rt quota at the same time Darrick J. Wong
2021-01-28 9:49 ` Christoph Hellwig
2021-01-28 18:01 ` Darrick J. Wong
2021-01-28 18:10 ` Brian Foster [this message]
2021-01-28 18:52 ` Darrick J. Wong
2021-01-28 6:01 ` [PATCH 07/13] xfs: refactor common transaction/inode/quota allocation idiom Darrick J. Wong
2021-01-28 9:50 ` Christoph Hellwig
2021-01-28 18:22 ` Brian Foster
2021-01-28 6:01 ` [PATCH 08/13] xfs: allow reservation of rtblocks with xfs_trans_alloc_inode Darrick J. Wong
2021-01-28 9:51 ` Christoph Hellwig
2021-01-28 18:22 ` Brian Foster
2021-01-28 6:01 ` [PATCH 09/13] xfs: refactor reflink functions to use xfs_trans_alloc_inode Darrick J. Wong
2021-01-28 9:53 ` Christoph Hellwig
2021-01-28 18:06 ` Darrick J. Wong
2021-01-28 18:23 ` Brian Foster
2021-01-28 6:02 ` [PATCH 10/13] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
2021-01-28 9:55 ` Christoph Hellwig
2021-01-28 18:23 ` Brian Foster
2021-01-28 6:02 ` [PATCH 11/13] xfs: refactor inode creation transaction/inode/quota allocation idiom Darrick J. Wong
2021-01-28 9:57 ` Christoph Hellwig
2021-01-28 18:18 ` Darrick J. Wong
2021-01-28 18:23 ` Brian Foster
2021-01-28 6:02 ` [PATCH 12/13] xfs: move xfs_qm_vop_chown_reserve to xfs_trans_dquot.c Darrick J. Wong
2021-01-28 9:58 ` Christoph Hellwig
2021-01-28 18:23 ` Brian Foster
2021-01-28 6:02 ` [PATCH 13/13] xfs: clean up xfs_trans_reserve_quota_chown a bit Darrick J. Wong
2021-01-28 10:00 ` Christoph Hellwig
2021-01-28 18:23 ` Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210128181021.GE2619139@bfoster \
--to=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox