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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
--
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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux