On Wed, Feb 26, 2020 at 09:07:28AM -0800, Christoph Hellwig wrote:

On Wed, Feb 26, 2020 at 09:04:25AM -0800, Darrick J. Wong wrote:

quoted

quoted

@@ -456,15 +435,8 @@ iomap_readpages(struct address_space *mapping, struct list_head *pages,
 			unlock_page(ctx.cur_page);
 		put_page(ctx.cur_page);
 	}
-
-	/*
-	 * Check that we didn't lose a page due to the arcance calling
-	 * conventions..
-	 */
-	WARN_ON_ONCE(!ret && !list_empty(ctx.pages));
-	return ret;

After all the discussion about "if we still have ctx.cur_page we should
just stop" in v7, I'm surprised that this patch now doesn't say much of
anything, not even a WARN_ON()?

The code quoted above puts the cur_page reference.  By dropping the
odd refactoring patch there is no need to check for cur_page being
left as a special condition as that still is the normal loop exit
state and properly handled, just as in the original iomap code.

DOH.  Yes, yes it does.  Thanks for pointing that out. :)

/me hands himself another cup of coffee,
Reviewed-by: Darrick J. Wong <redacted>

--D