Re: [PATCH 03/22] xfs: move perag structure and setup to libxfs/xfs_ag.[ch]

Dave Chinner
2021-05-10T23:38:31Z

On Mon, May 10, 2021 at 03:26:55PM -0700, Darrick J. Wong wrote:
> On Thu, May 06, 2021 at 05:20:35PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Move the xfs_perag infrastructure to the libxfs files that contain
> > all the per AG infrastructure. This helps set up for passing perags
> > around all the code instead of bare agnos with minimal extra
> > includes for existing files.
.....
> > -int xfs_initialize_perag_data(struct xfs_mount *, xfs_agnumber_t);
> > +
> > +/* per-AG block reservation data structures*/
> > +struct xfs_ag_resv {
> > + /* number of blocks originally reserved here */
> > + xfs_extlen_t ar_orig_reserved;
> > + /* number of blocks reserved here */
> > + xfs_extlen_t ar_reserved;
> > + /* number of blocks originally asked for */
> > + xfs_extlen_t ar_asked;
> > +};
> > +
> > +/*
> > + * Per-ag incore structure, copies of information in agf and agi, to improve the
> > + * performance of allocation group selection.
> > + */
> > +typedef struct xfs_perag {
> > + struct xfs_mount *pag_mount; /* owner filesystem */
> > + xfs_agnumber_t pag_agno; /* AG this structure belongs to */
> > + atomic_t pag_ref; /* perag reference count */
> > + char pagf_init; /* this agf's entry is initialized */
> > + char pagi_init; /* this agi's entry is initialized */
> > + char pagf_metadata; /* the agf is preferred to be metadata */
> > + char pagi_inodeok; /* The agi is ok for inodes */
> > + uint8_t pagf_levels[XFS_BTNUM_AGF];
> > + /* # of levels in bno & cnt btree */
> > + bool pagf_agflreset; /* agfl requires reset before use */
> > + uint32_t pagf_flcount; /* count of blocks in freelist */
> > + xfs_extlen_t pagf_freeblks; /* total free blocks */
> > + xfs_extlen_t pagf_longest; /* longest free space */
> > + uint32_t pagf_btreeblks; /* # of blocks held in AGF btrees */
> > + xfs_agino_t pagi_freecount; /* number of free inodes */
> > + xfs_agino_t pagi_count; /* number of allocated inodes */
> > +
> > + /*
> > + * Inode allocation search lookup optimisation.
> > + * If the pagino matches, the search for new inodes
> > + * doesn't need to search the near ones again straight away
> > + */
> > + xfs_agino_t pagl_pagino;
> > + xfs_agino_t pagl_leftrec;
> > + xfs_agino_t pagl_rightrec;
> > +
> > + int pagb_count; /* pagb slots in use */
> > + uint8_t pagf_refcount_level; /* recount btree height */
> > +
> > + /* Blocks reserved for all kinds of metadata. */
> > + struct xfs_ag_resv pag_meta_resv;
> > + /* Blocks reserved for the reverse mapping btree. */
> > + struct xfs_ag_resv pag_rmapbt_resv;
> > +
> > + /* -- kernel only structures below this line -- */
> > +
> > + /*
> > + * Bitsets of per-ag metadata that have been checked and/or are sick.
> > + * Callers should hold pag_state_lock before accessing this field.
> > + */
> > + uint16_t pag_checked;
> > + uint16_t pag_sick;
> > + spinlock_t pag_state_lock;
> > +
> > + spinlock_t pagb_lock; /* lock for pagb_tree */
> > + struct rb_root pagb_tree; /* ordered tree of busy extents */
> > + unsigned int pagb_gen; /* generation count for pagb_tree */
> > + wait_queue_head_t pagb_wait; /* woken when pagb_gen changes */
> > +
> > + atomic_t pagf_fstrms; /* # of filestreams active in this AG */
> > +
> > + spinlock_t pag_ici_lock; /* incore inode cache lock */
> > + struct radix_tree_root pag_ici_root; /* incore inode cache root */
> > + int pag_ici_reclaimable; /* reclaimable inodes */
> > + unsigned long pag_ici_reclaim_cursor; /* reclaim restart point */
> > +
> > + /* buffer cache index */
> > + spinlock_t pag_buf_lock; /* lock for pag_buf_hash */
> > + struct rhashtable pag_buf_hash;
> > +
> > + /* for rcu-safe freeing */
> > + struct rcu_head rcu_head;
> > +
> > + /* background prealloc block trimming */
> > + struct delayed_work pag_blockgc_work;
> > +
> > + /*
> > + * Unlinked inode information. This incore information reflects
> > + * data stored in the AGI, so callers must hold the AGI buffer lock
> > + * or have some other means to control concurrency.
> > + */
> > + struct rhashtable pagi_unlinked_hash;
> > +} xfs_perag_t;
>
> I wonder, have you ported this to xfsprogs yet? How much of a mess is
> this going to create for things like libxfs-diff?

I've looked at it, not yet ported. I structured the code so that
userspace should just need to add a couple of "#ifdef
KERNEL"/"#endif /* KERNEL */" pairs around the structure definition
and the init function, and then all userspace sees is the common
parts of the structure. Then the custom code to initialise the
perags in libxfs/init.c in userspace can go away....

IOWs, I think the xfs-diff should only show up a couple of ifdefs
and nothing else.

> (Also would be nice to kill off xfs_perag_t, but maybe you did that at
> the end of the series already <shrug>)

No, I don't think I did. Should be trivial to do, there's only 4 or
5 of them in use. I'll do it as a separate patch.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com