Re: [PATCH 3/4] xfs: use blocks for counting length of buffers

Dave Chinner
2012-03-29T00:31:40Z

On Wed, Mar 28, 2012 at 11:26:39AM -0400, Christoph Hellwig wrote:
> > + tmp = (valuelen < BBTOB(bp->b_length))
> > + ? valuelen : BBTOB(bp->b_length);
>
> maybe use min or min_t here while you're at it?
>
> > - tmp = (valuelen < XFS_BUF_SIZE(bp)) ? valuelen :
> > - XFS_BUF_SIZE(bp);
> > + tmp = valuelen < BBTOB(bp->b_length) ? valuelen :
> > + BBTOB(bp->b_length);
>
> Same here.
>
> > xfs_buf_iomove(bp, 0, tmp, src, XBRW_WRITE);
> > - if (tmp < XFS_BUF_SIZE(bp))
> > - xfs_buf_zero(bp, tmp, XFS_BUF_SIZE(bp) - tmp);
> > + if (tmp < BBTOB(bp->b_length))
> > + xfs_buf_zero(bp, tmp, BBTOB(bp->b_length) - tmp);
>
> Also a local buf_len variable in this function would probably be useful.
>
> > - size_t b_buffer_length;/* size of buffer in bytes */
> > + size_t b_length; /* size of buffer in BBs */
>
> A count of blocks probably shold not be a size_t, but a uint.
>
> > TP_fast_assign(
> > __entry->dev = bp->b_target->bt_dev;
> > __entry->bno = bp->b_bn;
> > - __entry->buffer_length = bp->b_buffer_length;
> > + __entry->buffer_length = BBTOB(bp->b_length);
>
> Given that we print the bno in blocks it might make sense to print
> this as number of blocks, too? Change the description string to nblks
> in that case as well.

All makes sense. Will do.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com