On Sat, Jun 18, 2016 at 01:15:10PM -0700, Darrick J. Wong wrote:
> 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.
Ok, so because all these functions go away, I'll take this patch now
without the suggested cleanups so that you don't have to rework it.
....
> > > + 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.
This seems best - add the clean up to the later patches rather than
have to rework lots of patches because of minor mods to early
patches...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
