On 2016-02-21, at 10:24 PM, John David Anglin wrote:
> On 2016-02-21, at 7:53 PM, John David Anglin wrote:
>
>> Backtrace:
>> [<000000004046f4b0>] scsi_init_sgtable+0x70/0xb8
>> [<000000004046f564>] scsi_init_io+0x6c/0x220
>> [<000000000811c5c0>] sd_setup_read_write_cmnd+0x58/0x968 [sd_mod]
>> [<000000000811cf14>] sd_init_command+0x44/0x130 [sd_mod]
>> [<000000004046f81c>] scsi_setup_cmnd+0x104/0x1b0
>> [<000000004046fab8>] scsi_prep_fn+0x100/0x1a0
>> [<000000004035b9b0>] blk_peek_request+0x1b8/0x298
>> [<0000000040471028>] scsi_request_fn+0xf8/0xa90
>> [<0000000040357244>] __blk_run_queue+0x4c/0x70
>> [<00000000403802c4>] cfq_insert_request+0x2dc/0x580
>> [<0000000040356404>] __elv_add_request+0x1b4/0x300
>>
>> We have in blk-merge.c:
>>
>> else {
>> /*
>> * If the driver previously mapped a shorter
>> * list, we could see a termination bit
>> * prematurely unless it fully inits the sg
>> * table on each mapping. We KNOW that there
>> * must be more entries here or the driver
>> * would be buggy, so force clear the
>> * termination bit to avoid doing a full
>> * sg_init_table() in drivers for each command.
>> */
>> if (sg_is_last (*sg))
>> printk ("__blk_segment_map_sg: clearing termination bi
>> t\n");
>> sg_unmark_end(*sg);
>> *sg = sg_next(*sg);
>> BUG_ON (!*sg);
>> }
>>
>> The comment suggests there must be more entries...
>
> I'm thinking with the split the scsi driver needs to provide one or two extra entires in the sg list.
With the attached patch, I'm able to boot 4.2.0-rc2+ on linux-block at commit
54efd50bfd873e2dbf784e0b21a8027ba4299a3e.
I didn't try to optimize the number of extra entries but I know one is not enough.
I guess the puzzle is why the number of entries isn't calculated correctly in the first place.
Further, why does blk-merge believe that it's okay to go beyond the terminator? Clearly,
the magic number isn't always set, etc.
I added the WARN_ON so I'd know when we run off the end of the the list.
Dave
--
John David Anglin dave.anglin@xxxxxxxx
diff --git a/block/blk-merge.c b/block/blk-merge.c
index d9c3a75..8e2566b 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -327,6 +327,7 @@ new_segment:
* termination bit to avoid doing a full
* sg_init_table() in drivers for each command.
*/
+ WARN_ON(sg_is_last (*sg));
sg_unmark_end(*sg);
*sg = sg_next(*sg);
}
@@ -392,6 +393,9 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
if (rq->bio)
nsegs = __blk_bios_map_sg(q, rq->bio, sglist, &sg);
+ if (!sg)
+ return nsegs;
+
if (unlikely(rq->cmd_flags & REQ_COPY_USER) &&
(blk_rq_bytes(rq) & q->dma_pad_mask)) {
unsigned int pad_len =
@@ -415,8 +419,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
rq->extra_len += q->dma_drain_size;
}
- if (sg)
- sg_mark_end(sg);
+ sg_mark_end(sg);
return nsegs;
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b1a2631..b421f03 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -595,6 +595,11 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
BUG_ON(!nents);
+ /* Provide extra entries in case of split. */
+ nents += 8;
+ if (nents > SCSI_MAX_SG_SEGMENTS)
+ nents = SCSI_MAX_SG_SEGMENTS;
+
if (mq) {
if (nents <= SCSI_MAX_SG_SEGMENTS) {
sdb->table.nents = nents;
