Re: [PATCH 06/11] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
From: Christoph Hellwig <hch@infradead.org>
Date: 2021-01-19 07:10:38
quoted hunk ↗ jump to hunk
@@ -351,13 +351,14 @@ xfs_reflink_allocate_cow( bool convert_now) { struct xfs_mount *mp = ip->i_mount; + struct xfs_trans *tp; xfs_fileoff_t offset_fsb = imap->br_startoff; xfs_filblks_t count_fsb = imap->br_blockcount; - struct xfs_trans *tp; - int nimaps, error = 0; - bool found; xfs_filblks_t resaligned; xfs_extlen_t resblks = 0; + bool found; + bool quota_retry = false; + int nimaps, error = 0;
Any good reason for reshuffling the declarations here?
+ if (error) {
+ /* This function must return with the ILOCK held. */
+ xfs_ilock(ip, *lockmode);
+ return error;
+ }Ugg.
+ if (error) {
+ xfs_trans_cancel(*tpp);
+ *tpp = NULL;
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ }
+
+ /* We only allow one retry for EDQUOT/ENOSPC. */
+ if (*retry || (error != -EDQUOT && error != -ENOSPC)) {
+ *retry = false;
+ return error;
+ }Id really don't like the semantics where this wrapper unlocks the ilock. Keeping all the locking at one layer, which is the callers makes the code much easier to reason about
+ + /* Try to free some quota for this file's dquots. */ + err2 = xfs_blockgc_free_quota(ip, 0, retry); + if (err2) + return err2; + return *retry ? 0 : error; }
Why not have a should_retry helper for the callers and let them call xfs_blockgc_free_quota? That is a little more boilerplate code, but a lot less obsfucated.