On Tue, May 27, 2014 at 10:43:59AM +0200, Maurizio Lombardi wrote:
> 
> But now I'm suspicious of this part of commit 3979ef4dcf:
> 
>  failed:
>         bvec->bv_page = NULL;
>         bvec->bv_len = 0;
>         bvec->bv_offset = 0;
>         bio->bi_vcnt--;  <----------------
>         blk_recount_segments(q, bio);
>         return 0;
> 
> Is decreasing bi_vcnt sufficient to guarantee that blk_recount_segments()
> recalculates the correct number of physical segments?
> Looking at the __blk_recalc_rq_segments() it appears it may not be the case.
> 
> The question is how can we restore the correct number of physical segments in 
> case
> of failure without breaking anything...
>

If my hypothesis is correct, the following patch should trigger a kernel panic,
Jet Chen, can you try it and let me know whether the BUG_ON is hit or not?

diff --git a/block/bio.c b/block/bio.c
index 0443694..763868f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -701,6 +701,7 @@ static int __bio_add_page(struct request_queue *q, struct 
bio *bio, struct page
                          unsigned int max_sectors)
 {
        int retried_segments = 0;
+       unsigned int phys_segments_orig;
        struct bio_vec *bvec;
 
        /*
@@ -751,6 +752,9 @@ static int __bio_add_page(struct request_queue *q, struct 
bio *bio, struct page
        if (bio->bi_vcnt >= bio->bi_max_vecs)
                return 0;
 
+       blk_recount_segments(q, bio);
+       phys_segments_orig = bio->bi_phys_segments;
+
        /*
         * setup the new entry, we might clear it again later if we
         * cannot add the page
@@ -811,6 +815,7 @@ static int __bio_add_page(struct request_queue *q, struct 
bio *bio, struct page
        bvec->bv_offset = 0;
        bio->bi_vcnt--;
        blk_recount_segments(q, bio);
+       BUG_ON(phys_segments_orig != bio->bi_phys_segments);
        return 0;
 }


Regards,
Maurizio Lombardi 
--
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/

Reply via email to