* [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache @ 2012-08-03 6:11 Jeff Liu 2012-08-08 21:21 ` Mark Tinguely 2012-08-09 23:37 ` Dave Chinner 0 siblings, 2 replies; 4+ messages in thread From: Jeff Liu @ 2012-08-03 6:11 UTC (permalink / raw) To: xfs Refine xfs_seek_data() to check up data buffer offset from page cache for unwritten extents. Signed-off-by: Jie Liu <jeff.liu@oracle.com> --- 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; + } + + /* + * 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; + start = XFS_FSB_TO_B(mp, fsbno); + if (start >= isize) { + error = ENXIO; + goto out_unlock; + } } if (offset != file->f_pos) -- 1.7.4.1 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache 2012-08-03 6:11 [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache Jeff Liu @ 2012-08-08 21:21 ` Mark Tinguely 2012-08-09 23:37 ` Dave Chinner 1 sibling, 0 replies; 4+ messages in thread From: Mark Tinguely @ 2012-08-08 21:21 UTC (permalink / raw) To: jeff.liu; +Cc: xfs On 08/03/12 01:11, 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@oracle.com> > > --- Looks great. Reviewed-by: Mark Tinguely <tinguely@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache 2012-08-03 6:11 [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache Jeff Liu 2012-08-08 21:21 ` Mark Tinguely @ 2012-08-09 23:37 ` Dave Chinner 2012-08-10 8:17 ` Jie Liu 1 sibling, 1 reply; 4+ messages in thread From: Dave Chinner @ 2012-08-09 23:37 UTC (permalink / raw) To: Jeff Liu; +Cc: xfs 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@oracle.com> > > --- > 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@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache 2012-08-09 23:37 ` Dave Chinner @ 2012-08-10 8:17 ` Jie Liu 0 siblings, 0 replies; 4+ messages in thread From: Jie Liu @ 2012-08-10 8:17 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 08/10/12 07:37, Dave Chinner wrote: > 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@oracle.com> >> >> --- >> 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; > } It looks very nice to wrap up those check ups in loop. Thanks, -Jeff > >> + >> + /* >> + * 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. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-10 8:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-03 6:11 [PATCH v6 3/4] xfs: xfs_seek_data() refinement with lookup data buffer offset from page cache Jeff Liu 2012-08-08 21:21 ` Mark Tinguely 2012-08-09 23:37 ` Dave Chinner 2012-08-10 8:17 ` Jie Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox