Jens added unlikely() thinking that this was an error path. It's actually just the end of the iteration, so does not warrant an unlikely().
Fixes: 7bed6f3d08b7 ("block: Fix iterating over an empty bio with bio_for_each_folio_all") Cc: stable@vger.kernel.org Signed-off-by: Matthew Wilcox (Oracle) willy@infradead.org --- include/linux/bio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h index 875d792bffff..1518f1201ddd 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -286,7 +286,7 @@ static inline void bio_first_folio(struct folio_iter *fi, struct bio *bio, { struct bio_vec *bvec = bio_first_bvec_all(bio) + i;
- if (unlikely(i >= bio->bi_vcnt)) { + if (i >= bio->bi_vcnt) { fi->folio = NULL; return; }
On 1/19/24 9:34 AM, Matthew Wilcox (Oracle) wrote:
Jens added unlikely() thinking that this was an error path. It's actually just the end of the iteration, so does not warrant an unlikely().
This is because the previous fix (or my attempt at least) didn't do the i >= vcnt, it checked for an empty bio instead. Which then definitely did make it an error/unlikely path, but obviously this one is not.
The bio iterator stuff has gotten terribly unwieldy and complicated, and not very efficient either. But I guess that's a story for another investigation...
On 1/19/24 9:41 AM, Jens Axboe wrote:
On 1/19/24 9:34 AM, Matthew Wilcox (Oracle) wrote:
Jens added unlikely() thinking that this was an error path. It's actually just the end of the iteration, so does not warrant an unlikely().
This is because the previous fix (or my attempt at least) didn't do the i >= vcnt, it checked for an empty bio instead. Which then definitely did make it an error/unlikely path, but obviously this one is not.
Just out of curiosity, I did some branch profiling on just normal operations of on my box. Of the ~900K times we hit this path, 10% of them ended up in that branch, and 90% of them did not. While it's not an error path, that does seem rather unlikely. Sure, for single entries, it'll be hit 50% of the time, but for most normal IO it'd definitely be less than 50%, and as per above non-scientif profiling, it's around 10%.
linux-stable-mirror@lists.linaro.org
-
Jens Axboe -
Matthew Wilcox (Oracle)