Thread (8 messages) 8 messages, 3 authors, 2012-08-21

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help