On Fri, Aug 03, 2012 at 02:11:07PM +0800, Jeff Liu wrote:
> Refine xfs_seek_data() to check up data buffer offset from page cache for
> unwritten extents.
>
>
> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>
> ---
> fs/xfs/xfs_file.c | 77 ++++++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 59 insertions(+), 18 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index aff0c30..e852e52 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1187,8 +1187,6 @@ xfs_seek_data(
> struct inode *inode = file->f_mapping->host;
> struct xfs_inode *ip = XFS_I(inode);
> struct xfs_mount *mp = ip->i_mount;
> - struct xfs_bmbt_irec map[2];
> - int nmap = 2;
> loff_t uninitialized_var(offset);
> xfs_fsize_t isize;
> xfs_fileoff_t fsbno;
> @@ -1204,34 +1202,77 @@ xfs_seek_data(
> goto out_unlock;
> }
>
> - fsbno = XFS_B_TO_FSBT(mp, start);
> -
> /*
> * Try to read extents from the first block indicated
> * by fsbno to the end block of the file.
> */
> + fsbno = XFS_B_TO_FSBT(mp, start);
> end = XFS_B_TO_FSB(mp, isize);
> + for (;;) {
> + struct xfs_bmbt_irec map[2];
> + int nmap = 2;
>
> - error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
> - XFS_BMAPI_ENTIRE);
> - if (error)
> - goto out_unlock;
> + error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
> + XFS_BMAPI_ENTIRE);
> + if (error)
> + goto out_unlock;
>
> - /*
> - * Treat unwritten extent as data extent since it might
> - * contains dirty data in page cache.
> - */
> - if (map[0].br_startblock != HOLESTARTBLOCK) {
> - offset = max_t(loff_t, start,
> - XFS_FSB_TO_B(mp, map[0].br_startoff));
> - } else {
> + /* No extents at given offset, must be beyond EOF */
> + if (nmap == 0) {
> + error = ENXIO;
> + goto out_unlock;
> + }
> +
> + offset = start;
> + /* Landed in a data extent */
> + if (map[0].br_startblock == DELAYSTARTBLOCK ||
> + (map[0].br_state == XFS_EXT_NORM &&
> + !isnullstartblock(map[0].br_startblock)))
> + break;
> +
> + /*
> + * Landed in an unwritten extent, try to search data
> + * from page cache.
> + */
> + if (map[0].br_state == XFS_EXT_UNWRITTEN) {
> + if (xfs_find_get_desired_pgoff(inode, &map[0],
> + DATA_OFF, &offset))
> + break;
> + }
> +
> + /*
> + * map[0] is hole or its an unwritten extent but
> + * without data in page cache. Probably means that
> + * we are reading after EOF if nothing in map[1].
> + */
> if (nmap == 1) {
> error = ENXIO;
> goto out_unlock;
> }
>
> - offset = max_t(loff_t, start,
> - XFS_FSB_TO_B(mp, map[1].br_startoff));
> + /* We have two mappings, proceed to check map[1] */
> + offset = XFS_FSB_TO_B(mp, map[1].br_startoff);
> + if (map[1].br_startblock == DELAYSTARTBLOCK ||
> + (map[1].br_state == XFS_EXT_NORM &&
> + !isnullstartblock(map[1].br_startblock)))
> + break;
> +
> + if (map[1].br_state == XFS_EXT_UNWRITTEN) {
> + if (xfs_find_get_desired_pgoff(inode, &map[1],
> + DATA_OFF, &offset))
> + break;
> + }
That's now duplicate code, so a loop would be much better to make it
simpler to maintain an enhance in future:
for (i = 0; i < nmap; i++) {
offset = max_t(loff_t, start,
XFS_FSB_TO_B(mp, map[i].br_startoff));
/* Landed in a data extent */
if (map[i].br_startblock == DELAYSTARTBLOCK ||
(map[i].br_state == XFS_EXT_NORM &&
!isnullstartblock(map[i].br_startblock)))
break;
/*
* Landed in an unwritten extent, try to search data
* from page cache.
*/
if (map[i].br_state == XFS_EXT_UNWRITTEN) {
if (xfs_find_get_desired_pgoff(inode, &map[i],
DATA_OFF,
&offset))
break;
}
}
/*
* map[0] is hole or its an unwritten extent but
* without data in page cache. Probably means that
* we are reading after EOF if nothing in map[1].
*/
if (nmap == 1) {
error = ENXIO;
goto out_unlock;
}
> +
> + /*
> + * Nothing was found, proceed to the next round of search
> + * if reading offset not beyond or hit EOF.
> + */
> + fsbno = map[1].br_startoff + map[1].br_blockcount;
fsbno = map[i].br_startoff + map[i].br_blockcount;
> + start = XFS_FSB_TO_B(mp, fsbno);
> + if (start >= isize) {
> + error = ENXIO;
> + goto out_unlock;
> + }
> }
Otherwise this looks just fine.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|