All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chandan Rajendra <chandanrlinux@gmail.com>
To: linux-xfs@vger.kernel.org
Cc: Chandan Rajendra <chandanrlinux@gmail.com>,
	david@fromorbit.com, chandan@linux.ibm.com,
	darrick.wong@oracle.com, bfoster@redhat.com, amir73il@gmail.com
Subject: [PATCH V4 RESEND 0/7] xfsprogs: Fix log reservation calculation for xattr insert operation
Date: Mon, 24 Feb 2020 09:31:29 +0530	[thread overview]
Message-ID: <20200224040136.31078-1-chandanrlinux@gmail.com> (raw)

Log space reservation for xattr insert operation can be divided into two
parts,
1. Mount time
   - Inode
   - Superblock for accounting space allocations
   - AGF for accounting space used by count, block number, rmap and refcnt
     btrees.

2. The remaining log space can only be calculated at run time because,
   - A local xattr can be large enough to cause a double split of the dabtree.
   - The value of the xattr can be large enough to be stored in remote
     blocks. The contents of the remote blocks are not logged.

   The log space reservation could be,
   - 2 * XFS_DA_NODE_MAXDEPTH number of blocks. Additional XFS_DA_NODE_MAXDEPTH
     number of blocks are required if xattr is large enough to cause another
     split of the dabtree path from root to leaf block.
   - BMBT blocks for storing (2 * XFS_DA_NODE_MAXDEPTH) record
     entries. Additional XFS_DA_NODE_MAXDEPTH number of blocks are required in
     case of a double split of the dabtree path from root to leaf blocks.
   - Space for logging blocks of count, block number, rmap and refcnt btrees.

At present mount time log reservation includes block count required for a
single split of the dabtree. The dabtree block count is also taken into
account by xfs_attr_calc_size().

Also, AGF log space reservation isn't accounted for.

Due to the reasons mentioned above, log reservation calculation for xattr
insert operation gives an incorrect value.

Apart from the above, xfs_log_calc_max_attrsetm_res() passes byte count as
an argument to XFS_NEXTENTADD_SPACE_RES() instead of block count.

This patchset aims to fix this log space reservation calcuation bug.

Patches 1-2 and 4-6 refactor the existing code around
xfs_attr_calc_size(). Patches 3 and 7 change the logic to fix log
reservation calculation.

The patchset can also be obtained from
https://github.com/chandanr/xfsprogs-dev/tree/xfs-fix-attr-resv-calc-v4.

Changelog:
V1 -> V2:
1. Use convenience variables to reduce indentation of code.

V2 -> V3:
1. Introduce 'struct xfs_attr_set_resv' to collect various block size
   reservations when inserting an xattr.
2. Add xfs_calc_attr_res() to calculate the total log reservation to
   required when inserting an xattr.

V3 -> V4:
1. Rebase the patchset on top of Christoph's "Clean attr interface"
   patchset. 
2. Split the patchset into
   - Patches which refactor the existing calculation in
     xfs_attr_calc_size().
   - One patch which fixes the calculation inside
     xfs_attr_calc_size().
3. Fix indentation issues.
4. Pass attribute geometry pointer to xfs_attr_leaf_newentsize()
   instead of a pointer to 'struct xfs_mount'.

Chandan Rajendra (7):
  xfsprogs: Pass xattr name and value length explicitly to
    xfs_attr_leaf_newentsize
  xfsprogs: xfs_attr_calc_size: Use local variables to track individual
    space components
  xfsprogs: xfs_attr_calc_size: Calculate Bmbt blks only once
  xfsprogs: Introduce struct xfs_attr_set_resv
  xfsprogs: xfs_attr_calc_size: Explicitly pass mp, namelen and valuelen
    args
  xfsprogs: Make xfs_attr_calc_size() non-static
  xfsprogs: Fix log reservation calculation for xattr insert operation

 libxfs/xfs_attr.c       | 104 ++++++++++++++++++++++------------------
 libxfs/xfs_attr.h       |  18 +++++++
 libxfs/xfs_attr_leaf.c  |  39 ++++++++++-----
 libxfs/xfs_attr_leaf.h  |   3 +-
 libxfs/xfs_log_rlimit.c |  16 +++----
 libxfs/xfs_trans_resv.c |  50 ++++++++++---------
 libxfs/xfs_trans_resv.h |   2 +
 7 files changed, 138 insertions(+), 94 deletions(-)

-- 
2.19.1


             reply	other threads:[~2020-02-24  3:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-24  4:01 Chandan Rajendra [this message]
2020-02-24  4:01 ` [PATCH V4 RESEND 1/7] xfsprogs: Pass xattr name and value length explicitly to xfs_attr_leaf_newentsize Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 2/7] xfsprogs: xfs_attr_calc_size: Use local variables to track individual space components Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 3/7] xfsprogs: xfs_attr_calc_size: Calculate Bmbt blks only once Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 4/7] xfsprogs: Introduce struct xfs_attr_set_resv Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 5/7] xfsprogs: xfs_attr_calc_size: Explicitly pass mp, namelen and valuelen args Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 6/7] xfsprogs: Make xfs_attr_calc_size() non-static Chandan Rajendra
2020-02-24  4:01 ` [PATCH V4 RESEND 7/7] xfsprogs: Fix log reservation calculation for xattr insert operation Chandan Rajendra

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=20200224040136.31078-1-chandanrlinux@gmail.com \
    --to=chandanrlinux@gmail.com \
    --cc=amir73il@gmail.com \
    --cc=bfoster@redhat.com \
    --cc=chandan@linux.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.