Re: [PATCH v7 2/4] xfs: Introduce a helper routine to probe data or hole offset from page cache
From: Jie Liu <hidden>
Date: 2012-08-21 06:42:13
On 08/21/12 13:25, Dave Chinner wrote:
On Tue, Aug 21, 2012 at 12:54:52PM +0800, Jie Liu wrote:quoted
On 08/20/12 23:31, Mark Tinguely wrote:quoted
On 08/13/12 08:07, Jeff Liu wrote:quoted
helper routine to lookup data or hole offset from page cache for unwritten extents. Signed-off-by: Jie Liu<redacted> --- fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 213 insertions(+), 0 deletions(-) +STATIC bool +xfs_find_get_desired_pgoff( + struct inode *inode, + struct xfs_bmbt_irec *map, + unsigned int type, + loff_t *offset) +{...quoted
+ 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?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.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
Cheers, Dave.
_______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs