On Sun, Feb 23, 2020 at 9:28 AM Chandan Rajendra <chandanrlinux@xxxxxxxxx> wrote: > > 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. > > Presently, 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. > > To fix these issues, this commit changes xfs_attr_calc_size() to also > calculate the number of dabtree blocks that need to be logged. > > xfs_attr_set() uses the following values computed by xfs_attr_calc_size() > 1. The number of dabtree blocks that need to be logged. > 2. The number of remote blocks that need to be allocated. > 3. The number of dabtree blocks that need to be allocated. > 4. The number of bmbt blocks that need to be allocated. > 5. The total number of blocks that need to be allocated. > > ... to compute number of bytes that need to be reserved in the log. > > This commit also modifies xfs_log_calc_max_attrsetm_res() to invoke > xfs_attr_calc_size() to obtain the number of blocks to be logged which it uses > to figure out the total number of bytes to be logged. > > Signed-off-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx> Hi Chandan, It would have been useful to get this sort of overview in a cover letter instead of having to find it in the last patch. I suppose it is a coincident that this work ended up in our mailboxes together with Allison's delayed attr work, but it is interesting to know if the two works affect each other in any way. Thanks, Amir.
