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

Christoph Hellwig
2011-12-08T16:07:12Z

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@lst.de>