linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Allison Henderson <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v24 03/11] xfs: Set up infrastructure for log atrribute replay
Date: Wed, 1 Sep 2021 07:52:34 +1000	[thread overview]
Message-ID: <20210831215234.GU3657114@dread.disaster.area> (raw)
In-Reply-To: <20210824224434.968720-4-allison.henderson@oracle.com>

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@oracle.com>
> ---
>  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...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2021-08-31 21:52 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 22:44 [PATCH v24 00/11] Log Attribute Replay Allison Henderson
2021-08-24 22:44 ` [PATCH v24 01/11] xfs: Return from xfs_attr_set_iter if there are no more rmtblks to process Allison Henderson
2021-08-24 22:44 ` [PATCH v24 02/11] xfs: Capture buffers for delayed ops Allison Henderson
2021-08-27 11:33   ` Chandan Babu R
2021-08-31 18:03     ` Allison Henderson
2021-08-30 17:44   ` Darrick J. Wong
2021-08-31 17:50     ` Allison Henderson
2021-09-18  0:02       ` Darrick J. Wong
2021-08-24 22:44 ` [PATCH v24 03/11] xfs: Set up infrastructure for log atrribute replay Allison Henderson
2021-08-27 14:17   ` Chandan Babu R
2021-08-31 18:03     ` Allison Henderson
2021-08-31  0:48   ` Dave Chinner
2021-08-31 19:03     ` Allison Henderson
2021-08-31 21:52   ` Dave Chinner [this message]
2021-09-01  5:34     ` Allison Henderson
2021-08-24 22:44 ` [PATCH v24 04/11] xfs: Implement attr logging and replay Allison Henderson
2021-08-30  7:47   ` Chandan Babu R
2021-08-31 18:11     ` Allison Henderson
2021-08-24 22:44 ` [PATCH v24 05/11] RFC xfs: Skip flip flags for delayed attrs Allison Henderson
2021-08-30 10:15   ` Chandan Babu R
2021-08-31 18:11     ` Allison Henderson
2021-08-24 22:44 ` [PATCH v24 06/11] xfs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred Allison Henderson
2021-08-30 10:27   ` Chandan Babu R
2021-08-31 18:12     ` Allison Henderson
2021-09-01  3:47   ` Dave Chinner
2021-09-01  5:34     ` Allison Henderson
2021-08-24 22:44 ` [PATCH v24 07/11] xfs: Remove unused xfs_attr_*_args Allison Henderson
2021-08-24 22:44 ` [PATCH v24 08/11] xfs: Add log attribute error tag Allison Henderson
2021-08-24 22:44 ` [PATCH v24 09/11] xfs: Add larp debug option Allison Henderson
2021-08-30 12:03   ` Chandan Babu R
2021-08-31 18:12     ` Allison Henderson
2021-08-24 22:44 ` [PATCH v24 10/11] xfs: Merge xfs_delattr_context into xfs_attr_item Allison Henderson
2021-08-24 22:44 ` [PATCH v24 11/11] xfs: Add helper function xfs_attr_leaf_addname Allison Henderson
2021-08-30 14:17   ` Chandan Babu R
2021-08-31 18:13     ` Allison Henderson
2021-08-31  0:20 ` [PATCH v24 00/11] Log Attribute Replay Dave Chinner
2021-08-31 18:27   ` Allison Henderson
2021-09-01  6:29     ` Dave Chinner
2021-09-01  7:30 ` [PATCH 0/5] xfs: various logged attribute fixes Dave Chinner
2021-09-01  7:30   ` [PATCH 1/5] xfs: fix fallthrough annotations in xfs_attr_set_iter() Dave Chinner
2021-09-01  7:30   ` [PATCH 2/5] xfs: fix flags passed to kvmalloc() by xfs_attri_init() Dave Chinner
2021-09-01  7:30   ` [PATCH 3/5] xfs: hide log iovec alignment constraints Dave Chinner
2021-09-01  7:30   ` [PATCH 4/5] xfs: fix attribute log iovec sizing Dave Chinner
2021-09-01  7:30   ` [PATCH 5/5] [RFC] xfs: don't commit the first deferred transaction without intents Dave Chinner
2021-09-01 21:43   ` [PATCH 0/5] xfs: various logged attribute fixes Allison Henderson

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=20210831215234.GU3657114@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=allison.henderson@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --subject='Re: [PATCH v24 03/11] xfs: Set up infrastructure for log atrribute replay' \
    /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

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).