On 07/26/12 10:32, Jeff Liu wrote:
Search data buffer offset for given range from page cache.
Signed-off-by: Jie Liu<jeff.liu@xxxxxxxxxx>
...
This is the same as version 4.
It looks good.
There are a couple places the comment precedes the test and is not yet
valid:
offset = max_t(loff_t, start,
- XFS_FSB_TO_B(mp, map[1].br_startoff));
+ XFS_FSB_TO_B(mp, map[0].br_startoff));
+ if (map[0].br_state == XFS_EXT_NORM&&
+ !isnullstartblock(map[0].br_startblock))
+ break;
+ else {
+ /*
+ * Landed in an unwritten extent, try to lookup data
+ * buffer from the page cache before proceeding to
+ * check the next extent map. It's a hole if nothing
+ * was found.
+ */
or it could be a hole
+ if (map[0].br_startblock == DELAYSTARTBLOCK ||
+ map[0].br_state == XFS_EXT_UNWRITTEN) {
here is where it is unwritten extent
+ /* Probing page cache start from offset */
+ if (xfs_find_get_desired_pgoff(inode,&map[0],
+ DATA_OFF,&offset))
+ break;
+ }
+
+ /*
+ * Found a hole in map[0] and nothing in map[1].
+ * Probably means that we are reading after EOF.
+ */
here we did find a hole
+ if (nmap == 1) {
here is where there is nothing in map[1] and ...
+ error = ENXIO;
+ goto out_unlock;
+ }
+
+ /*
+ * We have two mappings, proceed to check map[1].
+ */
+ offset = max_t(loff_t, start,
+ XFS_FSB_TO_B(mp, map[1].br_startoff));
+ if (map[1].br_state == XFS_EXT_NORM&&
+ !isnullstartblock(map[1].br_startblock))
+ break;
+ else {
+ /*
+ * map[1] is also an unwritten extent, lookup
+ * data buffer from page cache now.
+ */
it could be unwritten or a hole
+ if (map[1].br_startblock == DELAYSTARTBLOCK ||
+ map[1].br_state == XFS_EXT_UNWRITTEN) {
here is where we know it is unwritten.
+ if (xfs_find_get_desired_pgoff(inode,
+ &map[1], DATA_OFF,&offset))
+ break;
+ }
+ }
+ }
I am being really picky, but comments really help a quick read of the code.
Reviewed-by: Mark Tinguely <tinguely@xxxxxxx>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
