On 8/31/21 2:52 PM, Dave Chinner wrote:
On Tue, Aug 24, 2021 at 03:44:26PM -0700, Allison Henderson wrote:
Currently attributes are modified directly across one or more
transactions. But they are not logged or replayed in the event of an
error. The goal of log attr replay is to enable logging and replaying
of attribute operations using the existing delayed operations
infrastructure. This will later enable the attributes to become part of
larger multi part operations that also must first be recorded to the
log. This is mostly of interest in the scheme of parent pointers which
would need to maintain an attribute containing parent inode information
any time an inode is moved, created, or removed. Parent pointers would
then be of interest to any feature that would need to quickly derive an
inode path from the mount point. Online scrub, nfs lookups and fs grow
or shrink operations are all features that could take advantage of this.
This patch adds two new log item types for setting or removing
attributes as deferred operations. The xfs_attri_log_item will log an
intent to set or remove an attribute. The corresponding
xfs_attrd_log_item holds a reference to the xfs_attri_log_item and is
freed once the transaction is done. Both log items use a generic
xfs_attr_log_format structure that contains the attribute name, value,
flags, inode, and an op_flag that indicates if the operations is a set
or remove.
Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
---
fs/xfs/Makefile | 1 +
fs/xfs/libxfs/xfs_attr.c | 5 +-
fs/xfs/libxfs/xfs_attr.h | 31 +++
fs/xfs/libxfs/xfs_defer.h | 2 +
fs/xfs/libxfs/xfs_log_format.h | 44 +++-
fs/xfs/libxfs/xfs_log_recover.h | 2 +
fs/xfs/scrub/common.c | 2 +
fs/xfs/xfs_attr_item.c | 453 ++++++++++++++++++++++++++++++++
Comment on the overall structure of this file now I've been trying
to navigate through it for a little while. It is structured like:
<some attri stuff>
<some attrd stuff>
static const struct xfs_item_ops xfs_attrd_item_ops = {...}
<some more attri stuff>
static const struct xfs_item_ops xfs_attri_item_ops = {...}
<some attri log recovery stuff>
<some attrd log recovery stuff>
IOWs, the attri and attrd functions are interleaved non-obvious
ways and that makes it hard to navigate around when trying to find
related information. It would make more sense to me to structure
this as:
<attri stuff>
<attri log recovery stuff>
<some attrd stuff>
<attrd log recovery stuff>
static const struct xfs_item_ops xfs_attri_item_ops = {...}
const struct xlog_recover_item_ops xlog_attri_item_ops = {...}
static const struct xfs_item_ops xfs_attrd_item_ops = {...}
const struct xlog_recover_item_ops xlog_attrd_item_ops = {...}
because then all the related functionality is grouped together. It
also puts all the ops structures together in the one place, so we
don't have to jump around all over the file when just looking at
what ops the items run at different times...
Sure, will make a note to re-arrange some of these in the next version
Allison
Cheers,
Dave.
