Re: [PATCH 08/20] xfs: add owner field to extent allocation and freeing

Brian Foster
2015-06-25T13:03:36Z

On Thu, Jun 25, 2015 at 07:13:06AM +1000, Dave Chinner wrote:
> On Wed, Jun 24, 2015 at 03:09:19PM -0400, Brian Foster wrote:
> > On Wed, Jun 03, 2015 at 04:04:45PM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > For the rmap btree to work, we have to fed the extent owner
> > > information to the the allocation and freeing functions. This
> > > information is what will end up in the rmap btree that tracks
> > > allocated extents. While we technically don't need the owner
> > > information when freeing extents, passing it allows us to validate
> > > that the extent we are removing from the rmap btree actually
> > > belonged to the owner we expected it to belong to.
> > >
> > > We also define a special set of owner values for internal metadata
> > > that would otherwise have no owner. This allows us to tell the
> > > difference between metadata owned by different per-ag btrees, as
> > > well as static fs metadata (e.g. AG headers) and internal journal
> > > blocks.
> > >
> > > There are also a couple of special cases we need to take care of -
> > > during EFI recovery, we don't actually know who the original owner
> > > was, so we need to pass a wildcard to indicate that we aren't
> > > checking the owner for validity. We also need special handling in
> > > growfs, as we "free" the space in the last AG when extending it, but
> > > because it's new space it has no actual owner...
> > >
> >
> > Any reason not to support passing the owner through the efi/efd log
> > structures? You've already plumbed it through the bmap_free struct. I
> > suppose that could make this a backwards incompatible feature rather
> > than read-only incompatible, though.
>
> That's an interesting idea that I didn't really consider.
>
> I'll have a think about it, along with a couple of other suggestions
> from Darrick (e.g. increasing the rmap record size and keeping
> owner-related location information (file offset) in it) as that will
> also impact on any changes to EFI/EFD structure.
>

Ok.. it's not really a problem, it just seems like a gap given that this
work presumably pushes the owner through the rest of the extent free
path for verification purposes. I think it would be a bit of a shame to
do that work and not have the same verification in the event of a crash,
assuming it can be done relatively cleanly.

> As for ro compat vs incompat, an EFI/EFD change would be a log
> incompat flag, so only be relevant if the log is dirty at mount
> time. Hence if the log was clean then the ro-compat flag would be
> used, and so a change of EFI/EFD format isn't a huge deal...
>

That makes sense. Hmm, I have yet to get through the rest of this series
so I'm not sure whether extent free results in any rmapbt modifications,
but if so, wouldn't we need that log incompat. flag anyways? Even if the
EFI/EFD doesn't change, how can we allow an older kernel to recover
extent free transactions if they don't know how to update the rmapbt
appropriately?

Brian

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs