Re: [PATCH 009/119] xfs: convert list of extents to free into a regular list

Darrick J. Wong
2016-06-18T20:15:48Z

On Fri, Jun 17, 2016 at 04:59:30AM -0700, Christoph Hellwig wrote:
> > {
> > + struct xfs_bmap_free_item *new; /* new element */
> > #ifdef DEBUG
> > xfs_agnumber_t agno;
> > xfs_agblock_t agbno;
> > @@ -597,17 +595,7 @@ xfs_bmap_add_free(
> > new = kmem_zone_alloc(xfs_bmap_free_item_zone, KM_SLEEP);
> > new->xbfi_startblock = bno;
> > new->xbfi_blockcount = (xfs_extlen_t)len;
> > + list_add(&new->xbfi_list, &flist->xbf_flist);
> > flist->xbf_count++;
>
> Please kill xbf_count while you're at it, it's entirely superflous.

The deferred ops conversion patch kills this off by moving the whole
"defer an op to the next transaction by logging redo items" logic
into a separate file and mechanism.

This patch is just a cleanup to reduce some of the open coded list ugliness
before starting on the rmap stuff. Once the deferred ops code lands, all
three of these functions go away.

> > @@ -617,14 +605,10 @@ xfs_bmap_add_free(
> > */
> > void
> > xfs_bmap_del_free(
> > - xfs_bmap_free_t *flist, /* free item list header */
> > - xfs_bmap_free_item_t *prev, /* previous item on list, if any */
> > - xfs_bmap_free_item_t *free) /* list item to be freed */
> > + struct xfs_bmap_free *flist, /* free item list header */
> > + struct xfs_bmap_free_item *free) /* list item to be freed */
>
> Which then also gets rid of the flist argument here.
>
> > @@ -634,17 +618,16 @@ xfs_bmap_del_free(
> > */
> > void
> > xfs_bmap_cancel(
> > + struct xfs_bmap_free *flist) /* list of bmap_free_items */
> > {
> > + struct xfs_bmap_free_item *free; /* free list item */
> >
> > if (flist->xbf_count == 0)
> > return;
> > + while (!list_empty(&flist->xbf_flist)) {
> > + free = list_first_entry(&flist->xbf_flist,
> > + struct xfs_bmap_free_item, xbfi_list);
>
> while ((free = list_first_entry_or_null(...))
>
> > + list_sort((*tp)->t_mountp, &flist->xbf_flist, xfs_bmap_free_list_cmp);
>
> Can you add a comment on why we are sorting the list?

We sort the list so that we process the freed extents in AG order to
avoid deadlocking.

I'll add a comment to the deferred ops code if there isn't one already.

--D

>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs