On Thu, May 27, 2021 at 01:38:44PM -0700, Darrick J. Wong wrote:

On Wed, May 19, 2021 at 10:13:17PM +1000, Dave Chinner wrote:

quoted

+As a result, permanent transactions only "regrant" reservation space during
+xfs_trans_commit() calls, while the physical log space reservation - tracked by
+the write head - is then reserved separately by a call to xfs_log_reserve()
+after the commit completes. Once the commit completes, we can sleep waiting for
+physical log space to be reserved from the write grant head, but only if one
+critical rule has been observed::
+
+	Code using permanent reservations must always log the items they hold
+	locked across each transaction they roll in the chain.
+
+"Re-logging" the locked items on every transaction roll ensures that the items
+the transaction chain is rolling are always relocated to the physical head of

This reads (to me) a little awkwardly.  One could ask if the transaction
chain itself is rolling the items?  Which is not really what's
happening.  How about:

"...ensures that the items attached to the transaction chain being
rolled are always relocated..."

Fixed.

quoted

-This relogging is also used to implement long-running, multiple-commit
-transactions.  These transaction are known as rolling transactions, and require
-a special log reservation known as a permanent transaction reservation. A
-typical example of a rolling transaction is the removal of extents from an
+A typical example of a rolling transaction is the removal of extents from an
 inode which can only be done at a rate of two extents per transaction because

Ignoring rt files, do we even have /that/ limit anymore?  Especially
considering the other patchset you just sent... :)

Well.... Remind me to make this update in that patch set if I
forget to do it. :P

With that one odd sentence up there reworked,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

Thx!

-Dave.
-- 
Dave Chinner
david@fromorbit.com