xfs
[Top] [All Lists]

Re: [PATCH 026/119] xfs: add owner field to extent allocation and freein

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 026/119] xfs: add owner field to extent allocation and freeing
From: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Date: Tue, 5 Jul 2016 23:44:10 -0700
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, vishal.l.verma@xxxxxxxxx, xfs@xxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160706040120.GA12670@dastard>
References: <146612627129.12839.3827886950949809165.stgit@xxxxxxxxxxxxxxxx> <146612643914.12839.17925699349002137545.stgit@xxxxxxxxxxxxxxxx> <20160706040120.GA12670@dastard>
User-agent: Mutt/1.5.24 (2015-08-30)
On Wed, Jul 06, 2016 at 02:01:20PM +1000, Dave Chinner wrote:
> On Thu, Jun 16, 2016 at 06:20:39PM -0700, Darrick J. Wong wrote:
> > For the rmap btree to work, we have to feed 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.
> ....
> 
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -1318,6 +1318,71 @@ typedef __be32 xfs_inobt_ptr_t;
> >   */
> >  #define    XFS_RMAP_CRC_MAGIC      0x524d4233      /* 'RMB3' */
> >  
> > +/*
> > + * Ownership info for an extent.  This is used to create reverse-mapping
> > + * entries.
> > + */
> > +#define XFS_OWNER_INFO_ATTR_FORK   (1 << 0)
> > +#define XFS_OWNER_INFO_BMBT_BLOCK  (1 << 1)
> > +struct xfs_owner_info {
> > +   uint64_t                oi_owner;
> > +   xfs_fileoff_t           oi_offset;
> > +   unsigned int            oi_flags;
> > +};
> > +
> > +static inline void
> > +xfs_rmap_ag_owner(
> > +   struct xfs_owner_info   *oi,
> > +   uint64_t                owner)
> > +{
> > +   oi->oi_owner = owner;
> > +   oi->oi_offset = 0;
> > +   oi->oi_flags = 0;
> > +}
> > +
> > +static inline void
> > +xfs_rmap_ino_bmbt_owner(
> > +   struct xfs_owner_info   *oi,
> > +   xfs_ino_t               ino,
> > +   int                     whichfork)
> > +{
> > +   oi->oi_owner = ino;
> > +   oi->oi_offset = 0;
> > +   oi->oi_flags = XFS_OWNER_INFO_BMBT_BLOCK;
> > +   if (whichfork == XFS_ATTR_FORK)
> > +           oi->oi_flags |= XFS_OWNER_INFO_ATTR_FORK;
> > +}
> > +
> > +static inline void
> > +xfs_rmap_ino_owner(
> > +   struct xfs_owner_info   *oi,
> > +   xfs_ino_t               ino,
> > +   int                     whichfork,
> > +   xfs_fileoff_t           offset)
> > +{
> > +   oi->oi_owner = ino;
> > +   oi->oi_offset = offset;
> > +   oi->oi_flags = 0;
> > +   if (whichfork == XFS_ATTR_FORK)
> > +           oi->oi_flags |= XFS_OWNER_INFO_ATTR_FORK;
> > +}
> 
> One of the things we've avaoided doing so far is putting functions
> like this into xfs_format.h. xfs_format.h is really just for the
> on disk format definitions, not the code to access/pack/unpack it.
> Hence I think think these sorts of functions need to be moved to
> xfs_rmap.h....

Yes.

I've already split xfs_rmap_btree.h into xfs_rmap.h (high level rmap
functions) and xfs_rmap_btree.h (low level btree functions) for the
realtime rmapbt code split.  Won't be difficult to move these over
from xfs_format.h.

Speaking of which, I've pushed that along to the point that the
kernel-side implementation is at pre-alpha eatmydata stage, check
works well enough that xfstests doesn't explode, and we can collect
rt rmaps for checking and rebuilding of the tree.  I'll try to finish
that tomorrow.

--D

> 
> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>