From this patchset, there's now at least four different fixes: tags referring to it in Linus's tree. The patchset itself was broken and intermediate states didn't compile, making bisection annoying. And there's many other callers of this function that are potentially getting bad results, along with other cousins like bio_clone_bioset that are still unfixed.
This was not a fun experience for a new maintainer. It turns out if you had created a fresh bcache on 4.14, like most of my tests do-- you're probably fine. And not all environments will experience problems. It went in a couple of weeks before I became maintainer, and isn't in the bcache code itself (and wasn't reviewed by anyone who worked on bcache, and was never sent to the linux-bcache mailing list.) I've learned some new things to add to my test suite, and I guess i need to read all the block diff to make sure people don't break things out from under me.
As a maintainer, isn't it then your right to refuse the patchset and demanding a clean patchset instead?
For stuff I accept, I try and make sure that intermediate states compile, but I can't promise I run a compile on every intermediate step or that my review eye is good enough to spot every wrongly-ordered-dependent-change. It would be nice if someone would throw hardware at building intermediate states of linux-next and sent nastygrams to maintainers/committers about it: might make people more careful.
I have a continuous integration tool for bcache in place, but it's more geared on end states-- compiling/checkpatch/test sequence.
linux git:(c2ee070fb003) cat patch.coci
@@
expression lhs;
expression rhs;
@@
- lhs->bi_bdev = rhs;
+ bio_set_dev(bio, rhs);
so it spotted the original missing bi_partno = in the same file. diff -u -p a/block/bio.c b/block/bio.c
--- a/block/bio.c
+++ b/block/bio.c
@@ -596,7 +596,7 @@ void __bio_clone_fast(struct bio *bio, s
* most users will be overriding ->bi_bdev with a new target,
* so we don't set nor calculate new physical/hw segment counts here
*/
- bio->bi_bdev = bio_src->bi_bdev;
+ bio_set_dev(bio, bio_src->bi_bdev);
bio_set_flag(bio, BIO_CLONED);
bio->bi_opf = bio_src->bi_opf;
bio->bi_write_hint = bio_src->bi_write_hint;
@@ -681,7 +681,7 @@ struct bio *bio_clone_bioset(struct bio
bio = bio_alloc_bioset(gfp_mask, bio_segments(bio_src), bs);
if (!bio)
return NULL;
- bio->bi_bdev = bio_src->bi_bdev;
+ bio_set_dev(bio, bio_src->bi_bdev);
bio->bi_opf = bio_src->bi_opf;
bio->bi_write_hint = bio_src->bi_write_hint;
bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector;
but it also spotted a missing assignment in the same file in the bio_clone_bioset function. i don't know enough about this functionality to know if the assignment is meant to be here as well. also, considering it is in the same file i would have thought if it was necessary it would have been noticed in the last patch. just thought i would let someone else who would have a better idea about how this works know.i also haven't checked the rest of the coccinelle output. i know it has generated a lot of _wrong_ code. like the actual patch looks like it has done something more intelligent than what coccinelle blindly did.
To quote what you replied to: "And there's many other callers of this function that are potentially getting bad results, along with other cousins like bio_clone_bioset that are still unfixed."
bcache doesn't call bio_clone_bioset. I put in the minimum change to fix the bcache regression, for quick review and acceptance. You're free to submit this to linux-block if you want.