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

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs