xfs
[Top] [All Lists]

Re: [PATCH 02/12] xfs: clean up buffer get/read call API

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 02/12] xfs: clean up buffer get/read call API
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 8 Dec 2011 11:07:10 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1323238703-13198-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1323238703-13198-1-git-send-email-david@xxxxxxxxxxxxx> <1323238703-13198-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Dec 07, 2011 at 05:18:13PM +1100, Dave Chinner wrote:
> -     bp->b_file_offset = range_base;
> +     bp->b_file_offset = blkno << BBSHIFT;
>       /*
>        * Set buffer_length and count_desired to the same value initially.
>        * I/O routines should use count_desired, which will be the same in
>        * most cases but may be reset (e.g. XFS recovery).
>        */
> -     bp->b_buffer_length = bp->b_count_desired = range_length;
> +     bp->b_buffer_length = bp->b_count_desired = numblks << BBSHIFT;
>       bp->b_flags = flags;
> +
> +     /* XXX: we have the block number. Why don't we just set it here? */
>       bp->b_bn = XFS_BUF_DADDR_NULL;

If you touch all this anyway I think we should kill one of b_file_offset
and b_bn.  Given that b_file_offset is never used outside of xfs_buf.c
and with your patch we're using the block number much more often than
the byte offset b_file_offset seems like the likely candidate to kill
to me.

Otherwise this looks good to me,

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

<Prev in Thread] Current Thread [Next in Thread>