Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow

From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org, darrick.wong@oracle.com
Subject: Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
Date: Thu, 13 Oct 2016 09:26:47 -0400	[thread overview]
Message-ID: <20161013132647.GB9339@bfoster.bfoster> (raw)
In-Reply-To: <20161013065432.GC10579@lst.de>

On Thu, Oct 13, 2016 at 08:54:32AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 12, 2016 at 10:13:08AM -0400, Brian Foster wrote:
> > > +	/*
> > > +	 * Update the inode delalloc counter now and wait to update the
> > > +	 * sb counters as we might have to borrow some blocks for the
> > > +	 * indirect block accounting.
> > > +	 */
> > > +	xfs_trans_reserve_quota_nblks(NULL, ip, -((long)del->br_blockcount), 0,
> > > +			isrt ? XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS);
> > > +	ip->i_delayed_blks -= del->br_blockcount;
> > > +
> > 
> > This appears to be fixed up later, but i_delayed_blks is accounted twice
> > as of this patch. It would be nice if we could avoid known breakage,
> > even if transient.
> 
> Where do we account for it the second time?
> 

Note again that this is transient and fixed up in a later patch (so you
won't see it looking at the code with the full series applied). But as
of this patch the following is possible:

  ...
  xfs_reflink_cancel_cow_blocks()
    i_delayed_blks -= blockcount
    xfs_bunmapi_cow()
      xfs_bmap_del_extent_delay()
        i_delayed_blks -= blockcount

Brian

> > > +	case 0:
> > > +		/*
> > > +		 * Deleting the middle of the extent.
> > > +		 *
> > > +		 * Distribute the original indlen reservation across the two
> > > +		 * new extents.  Steal blocks from the deleted extent if
> > > +		 * necessary. Stealing blocks simply fudges the fdblocks
> > > +		 * accounting in xfs_bunmapi().
> > > +		 */
> > > +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> > > +		got->br_blockcount = del->br_startoff - got->br_startoff;
> > > +
> > > +		got_indlen = xfs_bmap_worst_indlen(ip, got->br_blockcount);
> > > +		new_indlen = xfs_bmap_worst_indlen(ip, new.br_blockcount);
> > 
> > Doesn't look like new.br_blockcount is set until a few lines below.
> 
> Indeed.  Fixed for the next resend.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html