On 08/21/12 07:13, Dave Chinner wrote: > On Mon, Aug 13, 2012 at 09:07:58PM +0800, Jeff Liu wrote: >> helper routine to lookup data or hole offset from page cache for unwritten extents. >> >> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx> > .... >> + 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. Thanks for pointing this out. > >> + 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; >> + } > 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.... 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 > > Cheers, > > Dave. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs