On Wed, Mar 28, 2012 at 11:20:28AM -0400, Christoph Hellwig wrote:

Looks good.

Reviewed-by: Christoph Hellwig <hch@lst.de>


Minor comments below:

On Wed, Mar 28, 2012 at 04:19:16PM +1100, Dave Chinner wrote:

quoted

-	end = bp->b_file_offset + bp->b_buffer_length;
-	page_count = xfs_buf_btoc(end) - xfs_buf_btoct(bp->b_file_offset);
+	end = BBTOB(bp->b_bn) + bp->b_buffer_length;
+	page_count = xfs_buf_btoc(end) - xfs_buf_btoct(BBTOB(bp->b_bn));

Btw, xfs_buf_btoc and xfs_buf_btoct are more remoal candidates,
they actually make the code harder to read compared to using the
opencoded arithmetics..

Ok, I'll consider doing that in the next set of patches.

quoted

 	numbytes = numblks << BBSHIFT;
 
 	/* Check for IOs smaller than the sector size / not sector aligned */
 	ASSERT(!(numbytes < (1 << btp->bt_sshift)));
-	ASSERT(!(offset & (xfs_off_t)btp->bt_smask));
+	ASSERT(!(BBTOB(blkno) & (xfs_off_t)btp->bt_smask));

I don't think it makes sense to keep this assert, given that the
return value from BBTOB is aligned by defintion.

I kept them there for the 4k sector case. Basic Blocks are always
512 bytes, so for filesystems with sector sizes greater than 512
bytes we still need a sector alignment and size check.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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