在 2012-7-28，上午5:32， Mark Tinguely 写道：
> 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.
Thanks for your review. I'll fix above comments as well as them in xfs_seek_hole() to make patch better.
Thanks,
-Jeff
>
> Reviewed-by: Mark Tinguely <tinguely@xxxxxxx>
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
