[PATCH] xfs: check all buffers in xfs_check_page_type()

[Shaun Gosse]

> Is there any reason to scan all the buffers when we all we want is an indication that at least one is acceptable?
> Maybe there are generally not may buffers to a page to make it worthwhile.
>

The problem is the else doesn't check if acceptable is set before breaking. So it can quit early, not having found what it was looking for even though it was in one of the later buffers. Pre-mature optimization...

So the patch is good as is. It improves correctness.

If the optimization were desired, though, it could be:

+ else if (acceptable)
+   break 

In addition to the remove already posted, and with proper whitespace.

Fair warning: this is without looking at the code, just inferring from what's been written in those two emails. I'm hearing my CS101 professor in my head and thinking may not be worth a couple cycles at the risk of correctness or clarity (and suggesting code blind seems risky).

Actually, this could be the first condition. If the whole point of that loop is just to make sure to set acceptable if it can be set according to the rules on the basis of at least one of the buffers, then a starting:

+ if (acceptable)
+    break

With an annotation something like "/** Optimization: Quit when acceptable has been set. Loop has no other side-effects to worry about.*/" seems reasonable (and hopefully is true). And then it's clear from the code that's the point of the loop and how it works.

Cheers,
-Shaun