Hi Ming, On 29.05.2014 15:59, Ming Lei wrote: > The patch of "bio: modify __bio_add_page() to accept pages that > don't start a new segment" changes the way for adding one page > to bio: > > - previously by adding page after checking successfully > - now by trying to add page and recover if it fails > > Unfortunately the patch forgets to update bio->bi_iter.bi_size > before trying to add page, then the last vector for holding > the added page may not be covered if recouning segments is needed, > so bio->bi_phys_segments may become not consistent with the > actual bio page buffers after the page is added successfully > to the bio(after bi_iter.bi_size is added by 'len') > > Suppose the page in the last vector can't be merged to bio, tragedy > will happen when __bio_add_page() is called to add another page: > > - blk_recount_segments() is called and the actual segments get > figured out correctly > > - the actual segments may become queue_max_segments(q) plus one > in failure path > > - driver will find the segment count is too big to handle. > > The patch fixes the virtio-blk oops bug reported from Jet Chen in > below link: > > http://marc.info/?l=linux-kernel&m=140113053817095&w=2
A great patch description, thanks! :-) You can add: Tested-by: Dongsu Park <dongsu.p...@profitbricks.com> > Cc: Jens Axboe <ax...@kernel.dk> > Cc: Maurizio Lombardi <mlomb...@redhat.com> > Cc: Dongsu Park <dongsu.p...@profitbricks.com> > Cc: Christoph Hellwig <h...@lst.de> > Cc: Kent Overstreet <k...@daterainc.com> > Cc: Andrew Morton <a...@linux-foundation.org> > Reported-by: Jet Chen <jet.c...@intel.com> > Tested-by: Jet Chen <jet.c...@intel.com> > Signed-off-by: Ming Lei <ming....@canonical.com> > --- > Andrew, could you put the patch in your -mm tree > because the previous two patches were routed from > your tree? > > block/bio.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/block/bio.c b/block/bio.c > index 0443694..f9bae56 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -744,6 +744,7 @@ static int __bio_add_page(struct request_queue *q, struct > bio *bio, struct page > } > } > > + bio->bi_iter.bi_size += len; > goto done; > } > } > @@ -761,6 +762,7 @@ static int __bio_add_page(struct request_queue *q, struct > bio *bio, struct page > bvec->bv_offset = offset; > bio->bi_vcnt++; > bio->bi_phys_segments++; > + bio->bi_iter.bi_size += len; > > /* > * Perform a recount if the number of segments is greater > @@ -802,7 +804,6 @@ static int __bio_add_page(struct request_queue *q, struct > bio *bio, struct page > bio->bi_flags &= ~(1 << BIO_SEG_VALID); > > done: > - bio->bi_iter.bi_size += len; > return len; > > failed: > @@ -810,6 +811,7 @@ static int __bio_add_page(struct request_queue *q, struct > bio *bio, struct page > bvec->bv_len = 0; > bvec->bv_offset = 0; > bio->bi_vcnt--; > + bio->bi_iter.bi_size -= len; > blk_recount_segments(q, bio); > return 0; > } > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
