linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gao Xiang <hsiangkao@redhat.com>
To: linux-xfs@vger.kernel.org
Cc: Dave Chinner <david@fromorbit.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Brian Foster <bfoster@redhat.com>,
	Gao Xiang <hsiangkao@redhat.com>
Subject: [RFC PATCH v4 0/3] xfs: more unlinked inode list optimization v4
Date: Tue, 18 Aug 2020 21:30:12 +0800	[thread overview]
Message-ID: <20200818133015.25398-1-hsiangkao@redhat.com> (raw)
In-Reply-To: <20200724061259.5519-1-hsiangkao@redhat.com>

Hi forks,

This is RFC v4 version which is based on Dave's latest patchset:
 https://lore.kernel.org/r/20200812092556.2567285-1-david@fromorbit.com

I didn't send out v3 because it was based on Dave's previous RFC
patchset, but I'm still not quite sure to drop RFC tag since this
version is different from the previous versions...

Changes since v2:
 - rebase on new patchset, and omit the original first patch
   "xfs: arrange all unlinked inodes into one list" since it now
   has better form in the base patchset;

 - a tail xfs_inode pointer is no longer needed since the original
   patchset introduced list_head iunlink infrastructure and it can
   be used to get the tail inode;

 - take pag_iunlink_mutex lock until all iunlink log items are
   committed. Otherwise, xfs_iunlink_log() order would not be equal
   to the trans commit order so it can mis-reorder and cause metadata
   corruption I mentioned in v2.

   In order to archive that, some recursive count is introduced since
   there could be several iunlink operations in one transaction,
   and introduce some per-AG fields as well since these operations
   in the transaction may not operate inodes in the same AG. we may
   also need to take AGI buffer lock in advance (e.g. whiteout rename
   path) due to iunlink operations and locking order constraint.
   For more details, see related inlined comments as well...

 - "xfs: get rid of unused pagi_unlinked_hash" would be better folded
   into original patchset since pagi_unlinked_hash is no longer needed.

============

[Original text]

This RFC patchset mainly addresses the thoughts [*] and [**] from Dave's
original patchset,
https://lore.kernel.org/r/20200623095015.1934171-1-david@fromorbit.com

In short, it focues on the following ideas mentioned by Dave:
 - use bucket 0 instead of multiple buckets since in-memory double
   linked list finally works;

 - avoid taking AGI buffer and unnecessary AGI update if possible, so
   1) add a new lock and keep proper locking order to avoid deadlock;
   2) insert a new unlinked inode from the tail instead of head;

In addition, it's worth noticing 3 things:
 - xfs_iunlink_remove() should support old multiple buckets in order
   to keep old inode unlinked list (old image) working when recovering.

 - (but) OTOH, the old kernel recovery _shouldn't_ work with new image
   since the bucket_index from old xfs_iunlink_remove() is generated
   by the old formula (rather than keep in xfs_inode), which is now
   fixed as 0. So this feature is not forward compatible without some
   extra backport patches;

 - a tail xfs_inode pointer is also added in the perag, which keeps 
   track of the tail of bucket 0 since it's mainly used for xfs_iunlink().


The git tree is also available at
git://git.kernel.org/pub/scm/linux/kernel/git/xiang/linux.git tags/xfs/iunlink_opt_v4

Gitweb:
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/linux.git/log/?h=xfs/iunlink_opt_v4


Some preliminary tests are done (including fstests, but there seems
some pre-exist failures and I haven't looked into yet). And I confirmed
there was no previous metadata corruption mentioned in RFC v2 anymore.

To confirm that I'm in the right direction, I post the latest version
now since it haven't been updated for a while.

Comments and directions are welcomed. :)

Thanks,
Gao Xiang

Gao Xiang (3):
  xfs: get rid of unused pagi_unlinked_hash
  xfs: introduce perag iunlink lock
  xfs: insert unlinked inodes from tail

 fs/xfs/xfs_inode.c        | 194 ++++++++++++++++++++++++++++++++------
 fs/xfs/xfs_inode.h        |   1 +
 fs/xfs/xfs_iunlink_item.c |  16 ++++
 fs/xfs/xfs_mount.c        |   4 +
 fs/xfs/xfs_mount.h        |  14 +--
 5 files changed, 193 insertions(+), 36 deletions(-)

-- 
2.18.1


  parent reply	other threads:[~2020-08-18 13:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 13:57 [RFC PATCH 0/2] xfs: more unlinked inode list optimization v1 Gao Xiang
2020-07-07 13:57 ` [RFC PATCH 1/2] xfs: arrange all unlinked inodes into one list Gao Xiang
2020-07-08 22:33   ` Dave Chinner
2020-07-09  0:17     ` Gao Xiang
2020-07-07 13:57 ` [RFC PATCH 2/2] xfs: don't access AGI on unlinked inodes if it can Gao Xiang
2020-07-08 17:03   ` Darrick J. Wong
2020-07-08 23:40     ` Gao Xiang
2020-07-08 23:33   ` Dave Chinner
2020-07-09  0:55     ` Gao Xiang
2020-07-09  2:32       ` Dave Chinner
2020-07-09 10:36         ` Gao Xiang
2020-07-09 10:47           ` Gao Xiang
2020-07-09 22:36           ` Dave Chinner
2020-07-24  6:12 ` [RFC PATCH v2 0/3] xfs: more unlinked inode list optimization v2 Gao Xiang
2020-07-24  6:12   ` [RFC PATCH v2 1/3] xfs: arrange all unlinked inodes into one list Gao Xiang
2020-07-24  6:12   ` [RFC PATCH v2 2/3] xfs: introduce perag iunlink lock Gao Xiang
2020-07-24  6:12   ` [RFC PATCH v2 3/3] xfs: insert unlinked inodes from tail Gao Xiang
2020-08-18 13:30   ` Gao Xiang [this message]
2020-08-18 13:30     ` [RFC PATCH v4 1/3] xfs: get rid of unused pagi_unlinked_hash Gao Xiang
2020-08-19  0:54       ` Darrick J. Wong
2020-08-21  1:09         ` Dave Chinner
2020-08-18 13:30     ` [RFC PATCH v4 2/3] xfs: introduce perag iunlink lock Gao Xiang
2020-08-19  1:06       ` Darrick J. Wong
2020-08-19  1:23         ` Gao Xiang
2020-08-18 13:30     ` [RFC PATCH v4 3/3] xfs: insert unlinked inodes from tail Gao Xiang
2020-08-19  0:53     ` [RFC PATCH v4 0/3] xfs: more unlinked inode list optimization v4 Darrick J. Wong
2020-08-19  1:14       ` Gao Xiang
2020-08-20  2:46     ` Darrick J. Wong
2020-08-20  4:01       ` Gao Xiang

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=20200818133015.25398-1-hsiangkao@redhat.com \
    --to=hsiangkao@redhat.com \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).