From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>,
linux-xfs@vger.kernel.org, david@fromorbit.com
Subject: Re: [PATCH 06/11] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
Date: Mon, 25 Jan 2021 13:16:23 -0500 [thread overview]
Message-ID: <20210125181623.GL2047559@bfoster> (raw)
In-Reply-To: <20210124093953.GC670331@infradead.org>
On Sun, Jan 24, 2021 at 09:39:53AM +0000, Christoph Hellwig wrote:
> > + /* We only allow one retry for EDQUOT/ENOSPC. */
> > + if (*retry || (error != -EDQUOT && error != -ENOSPC)) {
> > + *retry = false;
> > + return error;
> > + }
>
> > + /* Release resources, prepare for scan. */
> > + xfs_trans_cancel(*tpp);
> > + *tpp = NULL;
> > + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > +
> > + /* Try to free some quota for this file's dquots. */
> > + *retry = true;
> > + xfs_blockgc_free_quota(ip, 0);
> > + return 0;
>
> I till have grave reservations about this calling conventions. And if
> you just remove the unlock and th call to xfs_blockgc_free_quota here
> we don't equire a whole lot of boilerplate code in the callers while
> making the code possible to reason about for a mere human.
>
I agree that the retry pattern is rather odd. I'm curious, is there a
specific reason this scanning task has to execute outside of transaction
context in the first place? Assuming it does because the underlying work
may involve more transactions or whatnot, I'm wondering if this logic
could be buried further down in the transaction allocation path.
For example, if we passed the quota reservation and inode down into a
new variant of xfs_trans_alloc(), it could acquire the ilock and attempt
the quota reservation as a final step (to avoid adding an extra
unconditional ilock cycle). If quota res fails, iunlock and release the
log res internally and perform the scan. From there, perhaps we could
retry the quota reservation immediately without logres or the ilock by
saving references to the dquots, and then only reacquire logres/ilock on
success..? Just thinking out loud so that might require further
thought...
Brian
next prev parent reply other threads:[~2021-01-25 18:18 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-23 18:51 [PATCHSET v4 00/11] xfs: try harder to reclaim space when we run out Darrick J. Wong
2021-01-23 18:52 ` [PATCH 01/11] xfs: refactor messy xfs_inode_free_quota_* functions Darrick J. Wong
2021-01-25 18:13 ` Brian Foster
2021-01-25 19:33 ` Darrick J. Wong
2021-01-23 18:52 ` [PATCH 02/11] xfs: don't stall cowblocks scan if we can't take locks Darrick J. Wong
2021-01-25 18:14 ` Brian Foster
2021-01-25 19:54 ` Darrick J. Wong
2021-01-26 13:14 ` Brian Foster
2021-01-26 18:34 ` Darrick J. Wong
2021-01-26 20:03 ` Brian Foster
2021-01-27 3:09 ` Darrick J. Wong
2021-01-23 18:52 ` [PATCH 03/11] xfs: xfs_inode_free_quota_blocks should scan project quota Darrick J. Wong
2021-01-25 18:14 ` Brian Foster
2021-01-23 18:52 ` [PATCH 04/11] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts Darrick J. Wong
2021-01-25 18:14 ` Brian Foster
2021-01-23 18:52 ` [PATCH 05/11] xfs: pass flags and return gc errors from xfs_blockgc_free_quota Darrick J. Wong
2021-01-24 9:34 ` Christoph Hellwig
2021-01-25 18:15 ` Brian Foster
2021-01-26 4:52 ` [PATCH v4.1 " Darrick J. Wong
2021-01-27 16:59 ` Christoph Hellwig
2021-01-27 17:11 ` Darrick J. Wong
2021-01-23 18:52 ` [PATCH 06/11] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
2021-01-24 9:39 ` Christoph Hellwig
2021-01-25 18:16 ` Brian Foster [this message]
2021-01-25 18:57 ` Darrick J. Wong
2021-01-26 13:26 ` Brian Foster
2021-01-26 21:12 ` Darrick J. Wong
2021-01-27 14:19 ` Brian Foster
2021-01-27 17:19 ` Darrick J. Wong
2021-01-26 4:53 ` [PATCH v4.1 " Darrick J. Wong
2021-01-23 18:52 ` [PATCH 07/11] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
2021-01-26 4:55 ` [PATCH v4.1 " Darrick J. Wong
2021-01-23 18:52 ` [PATCH 08/11] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
2021-01-26 4:55 ` [PATCH v4.1 " Darrick J. Wong
2021-01-23 18:52 ` [PATCH 09/11] xfs: add a tracepoint for blockgc scans Darrick J. Wong
2021-01-25 18:45 ` Brian Foster
2021-01-26 4:56 ` [PATCH v4.1 " Darrick J. Wong
2021-01-23 18:52 ` [PATCH 10/11] xfs: refactor xfs_icache_free_{eof,cow}blocks call sites Darrick J. Wong
2021-01-24 9:41 ` Christoph Hellwig
2021-01-25 18:46 ` Brian Foster
2021-01-26 2:33 ` Darrick J. Wong
2021-01-23 18:53 ` [PATCH 11/11] xfs: flush speculative space allocations when we run out of space Darrick J. Wong
2021-01-24 9:48 ` Christoph Hellwig
2021-01-25 18:46 ` Brian Foster
2021-01-25 20:02 ` Darrick J. Wong
2021-01-25 21:06 ` Brian Foster
2021-01-26 0:29 ` Darrick J. Wong
2021-01-27 16:57 ` Christoph Hellwig
2021-01-27 21:00 ` Darrick J. Wong
2021-01-26 4:59 ` [PATCH v4.1 " Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2021-01-28 6:02 [PATCHSET v5 00/11] xfs: try harder to reclaim space when we run out Darrick J. Wong
2021-01-28 6:02 ` [PATCH 06/11] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
2021-01-28 9:26 ` Christoph Hellwig
2021-01-18 22:11 [PATCHSET v3 00/11] xfs: try harder to reclaim space when we run out Darrick J. Wong
2021-01-18 22:12 ` [PATCH 06/11] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
2021-01-19 3:56 ` Darrick J. Wong
2021-01-19 7:08 ` Christoph Hellwig
2021-01-19 19:43 ` Darrick J. Wong
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=20210125181623.GL2047559@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;
as well as URLs for NNTP newsgroup(s).