On Tue, Feb 21, 2012 at 01:55:04AM -0500, Mike Snitzer wrote:
[..]
> I think the bio_has_data() change is at the heart of the BUG_ON().
>
> Now this branch in blk_rq_bio_prep() is no longer taken:
>
> if (bio_has_data(bio)) {
> rq->nr_phys_segments = bio_phys_segments(q, bio);
> rq->buffer = bio_data(bio);
> }
>
> This patch fixed the issue for me (though I'm still missing why
> bio->bi_phys_segments was 0 given blkdev_issue_write_same() sets it):
>
> Index: linux-2.6/drivers/scsi/sd.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/sd.c
> +++ linux-2.6/drivers/scsi/sd.c
> @@ -697,6 +697,7 @@ out:
> static int sd_setup_write_same_cmnd(struct scsi_device *sdp, struct request *rq)
> {
> struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
> + struct request_queue *q = sdkp->disk->queue;
> struct bio *bio = rq->bio;
> sector_t sector = bio->bi_sector;
> unsigned int nr_sectors = bio_sectors(bio);
> @@ -711,7 +712,8 @@ static int sd_setup_write_same_cmnd(stru
>
> rq->timeout = SD_WRITE_SAME_TIMEOUT;
> rq->__data_len = rq->resid_len = sdp->sector_size;
> - rq->nr_phys_segments = bio->bi_phys_segments;
> + rq->nr_phys_segments = bio_phys_segments(q, bio);
> + rq->buffer = bio_data(bio);
This kind of sounds not so good. We should have got it covered in
blk_rq_bio_prep().
First we returned "false" from bio_has_data() and skipped above assignment,
in blk_rq_bio_prep() and now we are trying to make up for it.
Maybe returning false from bio_has_data() for WRITE_SAME is not such a
good idea because this bio has one logical block of data.
Martin, thoughts?
Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
