diff mbox series

[1/2] xfs: document what LARP means

Message ID 170175456779.3910588.8343836136719400292.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded, archived
Headers show
Series xfs: elide defer work ->create_done if no intent | expand

Commit Message

Darrick J. Wong Dec. 5, 2023, 5:36 a.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Christoph requested a blurb somewhere explaining exactly what LARP
means.  I don't know of a good place other than the source code (debug
knobs aren't covered in Documentation/), so here it is.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_sysfs.c |    7 +++++++
 1 file changed, 7 insertions(+)

Comments

Christoph Hellwig Dec. 5, 2023, 5:38 a.m. UTC | #1 
On Mon, Dec 04, 2023 at 09:36:07PM -0800, Darrick J. Wong wrote:
> +/*
> + * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
> + * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
> + * V5 filesystems.  As a result, the intermediate progress of all setxattr and
> + * removexattr operations are tracked via the log and can be restarted during
> + * recovery.
> + */

Can you also add a sentence on why we even have this code and why you'd
want to set the flag?
Darrick J. Wong Dec. 5, 2023, 5:50 a.m. UTC | #2 
On Tue, Dec 05, 2023 at 06:38:42AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 04, 2023 at 09:36:07PM -0800, Darrick J. Wong wrote:
> > +/*
> > + * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
> > + * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
> > + * V5 filesystems.  As a result, the intermediate progress of all setxattr and
> > + * removexattr operations are tracked via the log and can be restarted during
> > + * recovery.
> > + */
> 
> Can you also add a sentence on why we even have this code and why you'd
> want to set the flag?

How about these last couple of sentences?

/*
 * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
 * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
 * V5 filesystems.  As a result, the intermediate progress of all setxattr and
 * removexattr operations are tracked via the log and can be restarted during
 * recovery.  This is useful for testing xattr recovery prior to merging of the
 * parent pointer feature which requires it to maintain consistency, and may be
 * enabled for userspace xattrs in the future.
 */

--D
Christoph Hellwig Dec. 5, 2023, 5:56 a.m. UTC | #3 
On Mon, Dec 04, 2023 at 09:50:28PM -0800, Darrick J. Wong wrote:
> How about these last couple of sentences?
> 
> /*
>  * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
>  * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
>  * V5 filesystems.  As a result, the intermediate progress of all setxattr and
>  * removexattr operations are tracked via the log and can be restarted during
>  * recovery.  This is useful for testing xattr recovery prior to merging of the
>  * parent pointer feature which requires it to maintain consistency, and may be
>  * enabled for userspace xattrs in the future.
>  */

Oooh.  So all the logged attrs work is preparation for parent pointers?
That makes a whole lot of sense, but I've missed it so far.  Yes, the
above comment is great.

With that:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Darrick J. Wong Dec. 5, 2023, 6:09 a.m. UTC | #4 
On Tue, Dec 05, 2023 at 06:56:42AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 04, 2023 at 09:50:28PM -0800, Darrick J. Wong wrote:
> > How about these last couple of sentences?
> > 
> > /*
> >  * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
> >  * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
> >  * V5 filesystems.  As a result, the intermediate progress of all setxattr and
> >  * removexattr operations are tracked via the log and can be restarted during
> >  * recovery.  This is useful for testing xattr recovery prior to merging of the
> >  * parent pointer feature which requires it to maintain consistency, and may be
> >  * enabled for userspace xattrs in the future.
> >  */
> 
> Oooh.  So all the logged attrs work is preparation for parent pointers?
> That makes a whole lot of sense, but I've missed it so far.  Yes, the
> above comment is great.

Yep!  And that's coming in the year end megapatchbomb. :)

--D

> With that:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index a3c6b1548723..59869a1ee49f 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -229,6 +229,13 @@  pwork_threads_show(
 }
 XFS_SYSFS_ATTR_RW(pwork_threads);
 
+/*
+ * The "LARP" (Logged extended Attribute Recovery Persistence) debugging knob
+ * sets the XFS_DA_OP_LOGGED flag on all xfs_attr_set operations performed on
+ * V5 filesystems.  As a result, the intermediate progress of all setxattr and
+ * removexattr operations are tracked via the log and can be restarted during
+ * recovery.
+ */
 static ssize_t
 larp_store(
 	struct kobject	*kobject,