From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from bombadil.infradead.org ([198.137.202.9]:59218 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753362AbcFQL7b (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Fri, 17 Jun 2016 07:59:31 -0400
Date: Fri, 17 Jun 2016 04:59:30 -0700
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: david@fromorbit.com, linux-fsdevel@vger.kernel.org,
	vishal.l.verma@intel.com, xfs@oss.sgi.com
Subject: Re: [PATCH 009/119] xfs: convert list of extents to free into a
 regular list
Message-ID: <20160617115930.GH19042@infradead.org>
References: <146612627129.12839.3827886950949809165.stgit@birch.djwong.org>
 <146612632997.12839.18026491074892368053.stgit@birch.djwong.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <146612632997.12839.18026491074892368053.stgit@birch.djwong.org>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

>  {
> +	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.

> @@ -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?