xfs: Introduce a helper routine to probe data or hole offset from page cache
PATCH v7 2/4 xfs: Introduce a helper routine to probe data or hole offset from page cache
helper routine to lookup data or hole offset from page cache for unwritten extents.
--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)
fs/xfs/xfs_file.c +213 -0
@@ -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,218 @@ xfs_vm_page_mkwrite(return block_page_mkwrite (vma, vmf, xfs_get_blocks); } +/* +* This type is designed to indicate the type of offset we would like +* to search from page cache for either xfs_seek_data() or xfs_seek_hole(). +*/ +enum { +HOLE_OFF =0 , +DATA_OFF , +}; + +/* +* Lookup the desired type of offset from the given page. +* +* On success, return true and the offset argument will point to the +* start of the region that was found. Otherwise this function will +* return false and keep the offset argument unchanged. +*/ +STATIC bool +xfs_lookup_buffer_offset ( +struct page *page, +loff_t *offset, +unsigned int type) +{ +loff_t lastoff =page_offset (page); +bool found = false; +struct buffer_head *bh, *head; + + bh = head =page_buffers (page); +do { +/* +* Unwritten extents that have data in the page +* cache covering them can be identified by the +* BH_Unwritten state flag. Pages with multiple +* buffers might have a mix of holes, data and +* unwritten extents - any buffer with valid +* data in it should have BH_Uptodate flag set +* on it. +*/ +if (buffer_unwritten (bh) || +buffer_uptodate (bh)) { +if (type ==DATA_OFF ) + found = true; + }else { +if (type ==HOLE_OFF ) + found = true; + } + +if (found) { + *offset = lastoff; +break ; + } + lastoff += bh->b_size; + }while ((bh = bh->b_this_page) != head); + +return found; +} + +/* +* This routine is called to find out and return a data or hole offset +* from the page cache for unwritten extents according to the desired +* type for xfs_seek_data() or xfs_seek_hole(). +* +* The argument offset is used to tell where we start to search from the +* page cache. Map is used to figure out the end points of the range to +* lookup pages. +* +* Return true if the desired type of offset was found, and the argument +* offset is filled with that address. Otherwise, return false and keep +* offset unchanged. +*/ +STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +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; +loff_t endoff; +loff_t startoff = *offset; +loff_t lastoff = startoff; +bool found = false; + +pagevec_init (&pvec,0 ); + + index = startoff >>PAGE_CACHE_SHIFT ; + endoff =XFS_FSB_TO_B (mp, map->br_startoff + map->br_blockcount); + end = endoff >>PAGE_CACHE_SHIFT ; +do { +int want; +unsigned nr_pages; +unsigned int i; + + want =min_t (pgoff_t, end - index, (pgoff_t )PAGEVEC_SIZE ); + nr_pages =pagevec_lookup (&pvec, inode->i_mapping, index, + want); +/* +* No page mapped into given range. If we are searching holes +* and if this is the first time we got into the loop, it means +* that the given offset is landed in a hole, return it. +* +* If we have already stepped through some block buffers to find +* holes but they all contains data. In this case, the last +* offset is already updated and pointed to the end of the last +* mapped page, if it does not reach the endpoint to search, +* that means there should be a hole between them. +*/ +if (nr_pages ==0 ) { +/* Data search found nothing */ +if (type ==DATA_OFF ) +break ; + +ASSERT (type ==HOLE_OFF ); +if (lastoff == startoff || lastoff < endoff) { + found = true; + *offset = lastoff; + } +break ; + } + +/* +* At lease we found one page. If this is the first time we +* step into the loop, and if the first page index offset is +* greater than the given search offset, a hole was found. +*/ +if (type ==HOLE_OFF && lastoff == startoff && + lastoff <page_offset (pvec.pages[0 ])) { + found = true; +break ; + } + +for (i =0 ; i < nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index > end) { +if (type ==HOLE_OFF && lastoff < endoff) { + *offset = lastoff; + found = true; + } + goto out; + } + +lock_page (page); +/* +* Page truncated or invalidated(page->mapping == NULL). +* We can freely skip it and proceed to check the next +* page. +*/ +if (unlikely (page->mapping != inode->i_mapping)) { +unlock_page (page); +continue ; + } + +if (!page_has_buffers (page)) { +unlock_page (page); +continue ; + } + + found =xfs_lookup_buffer_offset (page, &b_offset, type); +if (found) { +/* +* The found offset may be less than the start +* point to search if this is the first time to +* come here. +*/ + *offset =max_t (loff_t, startoff, b_offset); +unlock_page (page); + goto out; + } + +/* +* We either searching data but nothing was found, or +* searching hole but found a data buffer. In either +* case, probably the next page contains the desired +* things, update the last offset to it so. +*/ + lastoff =page_offset (page) +PAGE_SIZE ; + index = page->index +1 ; +unlock_page (page); + } + +/* +* The number of returned pages less than our desired, search +* done. In this case, nothing was found for searching data, +* but we found a hole behind the last offset. +*/ +if (nr_pages < want) { +if (type ==HOLE_OFF ) { + *offset = lastoff; + found = true; + } +break ; + } + +pagevec_release (&pvec); + }while (index <= end); + +out: +pagevec_release (&pvec); +return found; +} + STATIC loff_t xfs_seek_data( struct file *file,
Signature · 6 lines
1.7.4.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 14 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu<[email protected]>--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)fs/xfs/xfs_file.c +7
-0+STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +loff_t *offset) +{
...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite(
Quoted 11 lines
fs/xfs/xfs_file.c +11
-0+for (i =0 ; i< nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index> end) {
Shouldn't this sample of the index also be locked?
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1122
Quoted 8 lines
fs/xfs/xfs_file.c +8
-0+if (type ==HOLE_OFF && lastoff< endoff) { + *offset = lastoff; + found = true; + } + goto out; + } + +lock_page (page);
--Mark.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 18 lines · Mark Tinguely
Quoted 14 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu<[email protected]>--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)fs/xfs/xfs_file.c +7
-0+STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +loff_t *offset) +{...
Quoted 13 lines
fs/xfs/xfs_file.c xfs_vm_page_mkwrite(
Quoted 11 lines
fs/xfs/xfs_file.c +11
-0+for (i =0 ; i< nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index> end) {Shouldn't this sample of the index also be locked?
You can check it before the page is locked for being beyond EOF, but it still needs to be checked again after the page is locked as it can change between this check and the page being locked.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 32 lines · Mark Tinguely
Quoted 16 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu<[email protected]>--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)fs/xfs/xfs_file.c +7
-0+STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +loff_t *offset) +{...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite(
Quoted 11 lines
fs/xfs/xfs_file.c +11
-0+for (i =0 ; i< nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index> end) {
Quoted 1 line
Shouldn't this sample of the index also be locked?
Thanks for the review. Yes, it should be locked in concert with the sample of index below.
However, as I have mentioned at v6,
http://oss.sgi.com/archives/xfs/2012-08/msg00028.html
I really don't understand why page->index will be changed as those pages returned from pagevec_lookup() should
have refcount > 0. Hence, those pages can not be removed out of VM cache upon memory reclaim IMHO.
If so, both sample of index could be performed without page lock.
Thanks,
-Jeff
Quoted 10 lines
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1122
Quoted 9 lines
fs/xfs/xfs_file.c +8
-0+if (type ==HOLE_OFF && lastoff< endoff) { + *offset = lastoff; + found = true; + } + goto out; + } + +lock_page (page);
Quoted 6 lines
--Mark.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 37 lines · Jie Liu
Quoted 32 lines · Mark Tinguely
Quoted 16 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu<[email protected]>--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)fs/xfs/xfs_file.c +7
-0+STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +loff_t *offset) +{...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite(
Quoted 11 lines
fs/xfs/xfs_file.c +11
-0+for (i =0 ; i< nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index> end) {Quoted 1 line
Shouldn't this sample of the index also be locked?
Thanks for the review. Yes, it should be locked in concert with the sample of index below.
Quoted 6 lines
However, as I have mentioned at v6,
http://oss.sgi.com/archives/xfs/2012-08/msg00028.html
I really don't understand whypage->indexwill be changed as those pages returned frompagevec_lookup()should
have refcount > 0. Hence, those pages can not be removed out of VM cache upon memory reclaim IMHO.
Ah, true, you are right. It's been a while since I looked at the reference count vs truncate vs page locks in detail, and I have always tended to err on the side of caution. I'd suggest you need to copy the comment from write_cache_pages() here to remind us why it is safe to do the check unlocked, otherwise in a couple of years time someone will be asking themselves why this is safe... :/
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 42 lines · Dave Chinner
Quoted 35 lines · Jie Liu
Quoted 30 lines · Mark Tinguely
Quoted 16 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu<[email protected]>--- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-)fs/xfs/xfs_file.c +7
-0+STATIC bool +xfs_find_get_desired_pgoff ( +struct inode *inode, +struct xfs_bmbt_irec *map, +unsigned int type, +loff_t *offset) +{...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite(
Quoted 11 lines
fs/xfs/xfs_file.c +11
-0+for (i =0 ; i< nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index> end) {Quoted 1 line
Shouldn't this sample of the index also be locked?
Thanks for the review. Yes, it should be locked in concert with the sample of index below.
Quoted 6 lines
However, as I have mentioned at v6,
http://oss.sgi.com/archives/xfs/2012-08/msg00028.html
I really don't understand whypage->indexwill be changed as those pages returned frompagevec_lookup()should
have refcount > 0. Hence, those pages can not be removed out of VM cache upon memory reclaim IMHO.
Quoted 6 lines
Ah, true, you are right. It's been a while since I looked at the reference count vs truncate vs page locks in detail, and I have always tended to err on the side of caution. I'd suggest you need to copy the comment from
write_cache_pages()here to remind us why it is safe to do the check unlocked, otherwise in a couple of years time someone will be asking themselves why this is safe... :/
Thanks for your quick response and confirmation, I'll copy the comments so.
Thanks,
-Jeff
Quoted 4 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 3 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu <[email protected]>
....
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1066
Quoted 3 lines
fs/xfs/xfs_file.c +3
-0+unsigned int i; + + want =min_t (pgoff_t, end - index, (pgoff_t )PAGEVEC_SIZE );
No need for the cast to (pgoff_t) here. That's what the min_t() macro does for you.
Quoted 18 lines
fs/xfs/xfs_file.c +18
-0+for (i =0 ; i < nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index > end) { +if (type ==HOLE_OFF && lastoff < endoff) { + *offset = lastoff; + found = true; + } + goto out; + } +
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1131
Quoted 10 lines
fs/xfs/xfs_file.c +10
-0+lock_page (page); +/* +* Page truncated or invalidated(page->mapping == NULL). +* We can freely skip it and proceed to check the next +* page. +*/ +if (unlikely (page->mapping != inode->i_mapping)) { +unlock_page (page); +continue ; + }
Still need to check page->index again after locking the page. As this isn't performance critical, I don't think you need the check before the page is locked, anyway....
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Dave Chinner
Quoted 3 lines · Jeff Liu
helper routine to lookup data or hole offset from page cache for unwritten extents.
Signed-off-by Jie Liu <[email protected]>....
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1066
Quoted 3 lines
fs/xfs/xfs_file.c +3
-0+unsigned int i; + + want =min_t (pgoff_t, end - index, (pgoff_t )PAGEVEC_SIZE );No need for the cast to (
pgoff_t) here. That's what themin_t()macro does for you.
Thanks for pointing this out.
Quoted 24 lines
Quoted 24 lines
fs/xfs/xfs_file.c +23
-0+for (i =0 ; i < nr_pages; i++) { +struct page *page = pvec.pages[i]; +loff_t b_offset; + +/* +* Page index is out of range, searching done. +* If the current offset is not reaches the end +* of the specified search range, there should +* be a hole between them. +*/ +if (page->index > end) { +if (type ==HOLE_OFF && lastoff < endoff) { + *offset = lastoff; + found = true; + } + goto out; + } + +lock_page (page); +/* +* Page truncated or invalidated(page->mapping == NULL). +* We can freely skip it and proceed to check the next +* page.
Quoted 8 lines
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1131
Quoted 5 lines
fs/xfs/xfs_file.c +5
-0+*/ +if (unlikely (page->mapping != inode->i_mapping)) { +unlock_page (page); +continue ; + }Still need to check
page->indexagain after locking the page. As this isn't performance critical, I don't think you need the check before the page is locked, anyway....
I understand that we have to check page->mapping with page locked because race against inode truncation.
But could you teaching me why page->index will be changed in either memory reclaim or truncates even if
the page returned with refcount > 0?
Thanks,
-Jeff
Quoted 4 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs