[dm-devel] dm-io async WRITE_SAME results in iSCSI NULL pointer [was: Re: Write same support]
Mike Snitzer
snitzer at redhat.com
Tue Feb 21 06:55:04 UTC 2012
On Mon, Feb 20 2012 at 10:57pm -0500,
Martin K. Petersen <martin.petersen at oracle.com> wrote:
> >>>>> "Mike" == Mike Snitzer <snitzer at redhat.com> writes:
>
> Mike,
>
> Mike> One, thing I noticed: bio_has_data returns false for
> Mike> REQ_WRITE_SAME. But REQ_WRITE_SAME does have data, and it really
> Mike> should be accounted no?:
>
> I decided against it. We don't count discards either and write sames are
> not really page-out types of activity. Happy to change it if people
> think this is something we should handle. But what do we actually count?
> A single logical block or the number of sectors written by the target
> device?
I'd say the single logical block.
> Mike> That aside, I tried your updated code and hit this BUG when I use
> Mike> the patch that has always worked (my dm-thin patch that uses the
> Mike> blkdev_issue_write_same() interface):
>
> Mike> ------------[ cut here ]------------ kernel BUG at
> Mike> drivers/scsi/scsi_lib.c:1116!
>
> That's the
>
> BUG_ON(!req->nr_phys_segments);
>
> in scsi_setup_blk_pc_cmnd(). We set nr_phys_segments to bi_phys_segments
> just before calling that function. So how did you end up with a
> zero-segment bio?
All I did was apply this patch to your writesame2 branch:
http://people.redhat.com/msnitzer/patches/upstream/dm-io-WRITE_SAME/dm-thin-use-WRITE_SAME-for-zeroing.patch
(and run the thinp-test-suite test I referenced in the other mail).
-- I'm just using the blkdev_issue_write_same() interface.. nothing special
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);
memset(rq->cmd, 0, rq->cmd_len);
if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff) {
> PS. I was unsuccessful in getting the thinp test suite working. If you
> can come up with a simpler way for me to get DM to issue a write same
> then please share...
Any new block that gets provisioned will trigger zeroing (unless the
entire block will be consumed with data -- in that case the zero is
avoided). If you use the default thinp block size: a random IO
benchmark or a simple dd, of a partial block, should trigger zeroing.
Mike
More information about the dm-devel
mailing list