Re: [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment

[Christoph Hellwig <hch@infradead.org>]

On Tue, Dec 04, 2012 at 05:18:04PM -0600, Mark Tinguely wrote:
> Not every segment in a multi-segment buffer is dirty in a
> transaction and they will not be outputted. The assert in
> xfs_buf_item_format_segment() that checks for the at least
> one chunk of data in the segment to be used is not necessary
> true for multi-segmented buffers.
> 
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> ---
>  fs/xfs/xfs_buf_item.c |   20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> Index: b/fs/xfs/xfs_buf_item.c
> ===================================================================
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -287,6 +287,17 @@ xfs_buf_item_format_segment(
>  	 */
>  	base_size = offsetof(struct xfs_buf_log_format, blf_data_map) +
>  			(blfp->blf_map_size * sizeof(blfp->blf_data_map[0]));
> +
> +	nvecs = 0;
> +	first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
> +	if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
> +		/*
> +		 * If the map is not be dirty in the transaction, mark
> +		 * the size as zero and do not advance the vector pointer.
> +		 */
> +		goto out;
> +	}
> +

I don't really like the style of this check.  What's the problem of
doing it this way:

  1) fill out the first vecp
  2) do the stale check as-is
  3) handle the the first_bit == -1 case  ala:

	if (first_bit == -1) {
		blfp->blf_size = 0;
		return vecp;
	}

  4) only then increcement vecp

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