Re: [PATCH v6 2/4] xfs: Introduce a new function to find the desired type of offset from page cache

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Thu, Aug 09, 2012 at 09:41:05AM -0500, Mark Tinguely wrote:
.....
> >+
> >+            /*
> >+             * 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;
> >+            }
> 
> I played with the code and appreciate the trickiness of the startoff.
> I this can be refined a bit more. see below.
> 
> >+
> >+            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;
> >+                    }
> 
> Before we take the lock, we can check to see if the page starts
> later than expected (a hole). The pre-loop start check can be
> removed and something like the follow added:

Really, we don't need to be that tricky or smart. This is not a
performance critical operation, so we don't need to optimise away
page locks, especially when the risk of getting it wrong is
compromising data integrity...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx