From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Y. D." <duyuyang@gmail.com>
Subject: BUG in blkback? on bio generation
Date: Thu, 25 Dec 2008 15:44:52 +0800
Message-ID: <200812251544471829730@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain;
	charset="gb2312"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: xen-devel <xen-devel@lists.xensource.com>
Cc: Keir Fraser <keir.fraser@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

Hi,

Merry Christmas.

I am reading code in blkback.c, while I find that the bio generation might be really bad. 
The bio_add_page() returns length of added bio vector, so if it fails it returns 0.
Therefore, should not bio_add_page() be negated in the while() loop condition??

        for (i = 0; i < nseg; i++) {           
                if (((int)preq.sector_number|(int)seg[i].nsec) &
                    ((bdev_hardsect_size(preq.bdev) >> 9) - 1)) {
                        DPRINTK("Misaligned I/O request from domain %d",
                                blkif->domid);         
                        goto fail_put_bio;     
                }             

                while ((bio == NULL) ||           
          -------->(bio_add_page(bio,                           
                                     virt_to_page(vaddr(pending_req, i)),
                                     seg[i].nsec << 9,              
                                     seg[i].buf & ~PAGE_MASK) == 0)) {
                        bio = biolist[nbio++] = bio_alloc(GFP_KERNEL, nseg-i);
                        if (unlikely(bio == NULL))             
                                goto fail_put_bio;                                                                                                             

                        bio->bi_bdev    = preq.bdev;   
                        bio->bi_private = pending_req; 
                        bio->bi_end_io  = end_block_io_op;
                        bio->bi_sector  = preq.sector_number;                                                                                                  
                }             

                preq.sector_number += seg[i].nsec;
        }

Cheers,
Shawn