xfs: probe data buffer from page cache for unwritten extents
PATCH v3 1/1 xfs: probe data buffer from page cache for unwritten extents
Hello,
This is v3 of the patch.
We can trigger BUG() in xfs_seek_data() if met two unwritten without data or hole extents at last version. So making the extents map reading in loop could solve it.
Sorry, Am not yet try the repeated holes scenario according to Dave's comments as lack of X64 test env, still waiting for it ready. But this patch is already too long delayed, I have worked it out one weeks ago. So I'd like to post it because of it could handle repeated hole/unwritten extents well in a loop, and I also improved xfstests:286 with those cases for the verification, will post it soon.
v2->v3:
Tested by Mark, hit BUG() for continuous unwritten extents without data wrote.
* xfs_seek_data(), remove BUG() and having extents map search in loop.
v1->v2:
suggested by Mark.
* xfs_has_unwritten_buffer(), use the input offset instead of bmap->br_startoff to calculate page index for data buffer probing.
Thanks,
-Jeff
--- fs/xfs/xfs_file.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 178 insertions(+), 19 deletions(-)
fs/xfs/xfs_file.c +178 -19
@@ -36,6 +36,7 @@#include <linux/dcache.h> #include <linux/falloc.h> +#include <linux/pagevec.h> static const struct vm_operations_struct xfs_file_vm_ops;
@@ -966,6 +967,108 @@ xfs_vm_page_mkwrite(return block_page_mkwrite (vma, vmf, xfs_get_blocks); } +/* +* Probe the data buffer offset in page cache for unwritten extents. +* Iterate each page to find out if a buffer head state has BH_Unwritten +* or BH_Uptodate set. +*/ +STATIC bool +xfs_has_unwritten_buffer ( +struct inode *inode, +struct xfs_bmbt_irec *map, +loff_t *offset) +{ +struct xfs_inode *ip =XFS_I (inode); +struct xfs_mount *mp = ip->i_mount; +struct pagevec pvec; +pgoff_t index; +pgoff_t end; +bool found = false; + +pagevec_init (&pvec,0 ); + + index =XFS_FSB_TO_B (mp,XFS_B_TO_FSBT (mp, *offset)) + >>PAGE_CACHE_SHIFT ; + end =XFS_FSB_TO_B (mp, map->br_startoff + map->br_blockcount) + >>PAGE_CACHE_SHIFT ; +do { +unsigned int i; +unsigned nr_pages; +int want =min_t (pgoff_t, end - index, + (pgoff_t )PAGEVEC_SIZE -1 ) +1 ; + nr_pages =pagevec_lookup (&pvec, inode->i_mapping, index, + want); +if (nr_pages ==0 ) +break ; + +for (i =0 ; i < nr_pages; i++) { +struct page *page = pvec.pages[i]; +struct buffer_head *bh; +struct buffer_head *head; +xfs_fileoff_t last; + +/* +* There is no need to check the following pages +* if the current page offset is out of range. +*/ +if (page->index > end) + goto out; + +if (!trylock_page (page)) + goto out; + +if (!page_has_buffers (page)) { +unlock_page (page); +continue ; + } + + last =XFS_B_TO_FSBT (mp, + page->index <<PAGE_CACHE_SHIFT ); + bh = head =page_buffers (page); +do { +/* +* An extent in XFS_EXT_UNWRITTEN has disk +* blocks already mapped to it, but no data +* has been committed to them yet. If it has +* dirty data in the page cache it can be +* identified by having BH_Unwritten set in +* each buffers. Also, the buffer head state +* might be in BH_Uptodate too if the buffer +* writeback procedure was fired, we need to +* examine it as well. +*/ +if ((buffer_unwritten (bh) || +buffer_uptodate (bh)) && + *offset <=XFS_FSB_TO_B (mp, last)) { + found = true; + *offset =XFS_FSB_TO_B (mp, last); +unlock_page (page); + goto out; + } + last++; + }while ((bh = bh->b_this_page) != head); +unlock_page (page); + } + +/* +* If the number of probed pages less than our desired, +* there should no more pages mapped, search done. +*/ +if (nr_pages < want) +break ; + + index = pvec.pages[i -1 ]->index +1 ; +pagevec_release (&pvec); + }while (index < end); + +out: +pagevec_release (&pvec); +if (!found) + *offset =0 ; + +return found; +} + STATIC loff_t xfs_seek_data( struct file *file,
@@ -975,8 +1078,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;
@@ -992,36 +1093,94 @@ 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 { -if (nmap ==1 ) { +/* No extents at given offset, must be beyond EOF */ +if (nmap ==0 ) { error =ENXIO ; goto out_unlock; } - offset =max_t (loff_t, start, -XFS_FSB_TO_B (mp, map[1 ].br_startoff)); + offset = start; +if (map[0 ].br_state ==XFS_EXT_NORM && + !isnullstartblock (map[0 ].br_startblock)) +break ; +else { +/* +* Landed in an unwritten extent, try to search +* data buffer from page cache firstly. Treat +* it as a hole if nothing was found, and skip +* to check the next extent. +*/ +if (map[0 ].br_startblock ==DELAYSTARTBLOCK || + map[0 ].br_state ==XFS_EXT_UNWRITTEN ) { +/* Probing page cache start from offset */ +if (xfs_has_unwritten_buffer (inode, &map[0 ], + &offset)) + goto out; + } + +/* +* Found a hole in map[0] and nothing in map[1]. +* Probably means that we are reading after EOF. +*/ +if (nmap ==1 ) { + error =ENXIO ; + goto out_unlock; + } + +/* +* We have two mappings, and need to check map[1] to +* see if there is data or not. +*/ + 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)) + goto out; +else { +/* +* So map[1] is an unwritten extent as well, +* try to search for data buffer in page cache. +* We might find nothing if it is an allocated +* and resvered extent. +*/ +if (map[1 ].br_startblock ==DELAYSTARTBLOCK || + map[1 ].br_state ==XFS_EXT_UNWRITTEN ) { +if (xfs_has_unwritten_buffer (inode, + &map[1 ], &offset)) + goto out; + } + } + } + +/* +* Nothing was found, proceed to the next round of search if +* reading offset not beyond 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; + } } +out:if (offset != file->f_pos) file->f_pos = offset;
Signature · 6 lines
1.7.9 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 21 lines · Jeff Liu
Hello,
This is v3 of the patch.
We can trigger
BUG()inxfs_seek_data()if met two unwritten without data or hole extents at last version. So making the extents map reading in loop could solve it.Sorry, Am not yet try the repeated holes scenario according to Dave's comments as lack of X64 test env, still waiting for it ready. But this patch is already too long delayed, I have worked it out one weeks ago. So I'd like to post it because of it could handle repeated hole/unwritten extents well in a loop, and I also improved xfstests:286 with those cases for the verification, will post it soon.
v2->v3:
Tested by Mark, hitBUG()for continuous unwritten extents without data wrote.
*xfs_seek_data(), removeBUG()and having extents map search in loop.
v1->v2:
suggested by Mark.
*xfs_has_unwritten_buffer(), use the input offset instead ofbmap->br_startoffto calculate page index for data buffer probing.
Quoted 7 lines
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>---
Nice. Up to refining xfs_seek_hole()?
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Jeff Liu
Hello,
This is v3 of the patch.
We can trigger
BUG()inxfs_seek_data()if met two unwritten without data or hole extents at last version. So making the extents map reading in loop could solve it.
Quoted 8 lines
Sorry, Am not yet try the repeated holes scenario according to Dave's comments as lack of X64 test env, still waiting for it ready. But this patch is already too long delayed, I have worked it out one weeks ago. So I'd like to post it because of it could handle repeated hole/unwritten extents well in a loop, and I also improved xfstests:286 with those cases for the verification, will post it soon.
v2->v3:
Tested by Mark, hitBUG()for continuous unwritten extents without data wrote.
*xfs_seek_data(), removeBUG()and having extents map search in loop.
The patch looks good. But as question by Mark I wonder if it's a good idea to just improve xfs_seek_data, but not xfs_seek_hole.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 13 lines · Christoph Hellwig
Quoted 12 lines · Jeff Liu
Hello,
This is v3 of the patch.
We can trigger
BUG()inxfs_seek_data()if met two unwritten without data or hole extents at last version. So making the extents map reading in loop could solve it.Sorry, Am not yet try the repeated holes scenario according to Dave's comments as lack of X64 test env, still waiting for it ready. But this patch is already too long delayed, I have worked it out one weeks ago. So I'd like to post it because of it could handle repeated hole/unwritten extents well in a loop, and I also improved xfstests:286 with those cases for the verification, will post it soon.
Quoted 6 lines
Quoted 3 lines
v2->v3:
Tested by Mark, hitBUG()for continuous unwritten extents without data wrote.
*xfs_seek_data(), removeBUG()and having extents map search in loop.The patch looks good. But as question by Mark I wonder if it's a good idea to just improve
xfs_seek_data, but notxfs_seek_hole.
I definitely would like to improve it.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 26 lines · Mark Tinguely
Quoted 25 lines · Jeff Liu
Hello,
This is v3 of the patch.
We can trigger
BUG()inxfs_seek_data()if met two unwritten without data or hole extents at last version. So making the extents map reading in loop could solve it.Sorry, Am not yet try the repeated holes scenario according to Dave's comments as lack of X64 test env, still waiting for it ready. But this patch is already too long delayed, I have worked it out one weeks ago. So I'd like to post it because of it could handle repeated hole/unwritten extents well in a loop, and I also improved xfstests:286 with those cases for the verification, will post it soon.
v2->v3:
Tested by Mark, hitBUG()for continuous unwritten extents without data wrote.
*xfs_seek_data(), removeBUG()and having extents map search in loop.
v1->v2:
suggested by Mark.
*xfs_has_unwritten_buffer(), use the input offset instead ofbmap->br_startoffto calculate page index for data buffer probing.Thanks,
-Jeff
Quoted 5 lines
Quoted 3 lines
Signed-off-by Jie Liu<[email protected]>---
Nice. Up to refining
xfs_seek_hole()?
Thanks for the review.
Sorry for my late response, I missed you this email. Sure, the refinement will be reflected in next post. :)
Thanks,
-Jeff
Quoted 8 lines
Reviewed-by Mark Tinguely <[email protected]>_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs