From patchwork Tue Feb 23 03:04:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John David Anglin X-Patchwork-Id: 8386441 Return-Path: X-Original-To: patchwork-linux-parisc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 07756C0553 for ; Tue, 23 Feb 2016 03:04:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 38A1A205D4 for ; Tue, 23 Feb 2016 03:04:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 43C0A205C6 for ; Tue, 23 Feb 2016 03:04:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755997AbcBWDEM (ORCPT ); Mon, 22 Feb 2016 22:04:12 -0500 Received: from belmont79srvr.owm.bell.net ([184.150.200.79]:42636 "EHLO mtlfep01.bell.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754795AbcBWDEL (ORCPT ); Mon, 22 Feb 2016 22:04:11 -0500 Received: from bell.net mtlfep01 184.150.200.30 by mtlfep01.bell.net with ESMTP id <20160223030410.FJWP19982.mtlfep01.bell.net@mtlspm02.bell.net> for ; Mon, 22 Feb 2016 22:04:10 -0500 Received: from [192.168.2.10] (really [70.54.51.120]) by mtlspm02.bell.net with ESMTP id <20160223030409.NREW21522.mtlspm02.bell.net@[192.168.2.10]>; Mon, 22 Feb 2016 22:04:09 -0500 Subject: Re: SCSI bug Mime-Version: 1.0 (Apple Message framework v1085) From: John David Anglin In-Reply-To: <951EFC7D-4166-41E5-BD1B-E22EE7884AD7@bell.net> Date: Mon, 22 Feb 2016 22:04:09 -0500 Cc: Helge Deller , James Bottomley , linux-parisc List Message-Id: <38C40D56-D72B-4F87-AAF3-050391B9546D@bell.net> References: <28D5FC66-A9E6-4F3E-A8AC-F9E68A6BC543@bell.net> <82D4BF6F-05CF-4441-9530-79475BFF84F3@bell.net> <56C8E1AC.3030409@gmx.de> <038473FE-9E6D-4F03-BBD4-9844B7B529E1@bell.net> <9C42B388-96D5-412A-BAC3-14E93415CA21@bell.net> <1456026424.2268.5.camel@HansenPartnership.com> <14EF1F4E-8045-4990-B29E-D489E82B1929@bell.net> <1456078381.2340.5.camel@HansenPartnership.com> <1456081676.2340.10.camel@HansenPartnership.com> <56CA11D2.3020900@gmx.de> <1456086490.2340.18.camel@HansenPartnership.com> <28582474-5AB3-4022-86FE-E823F5990623@bell.net> <56CA2982.1030607@gmx.de> <0905D3D6-635F-4E09-BFD0-2EBE5DA0B0CC@bell.net> <951EFC7D-4166-41E5-BD1B-E22EE7884AD7@bell.net> To: John David Anglin X-Mailer: Apple Mail (2.1085) X-Opwv-CommTouchExtSvcRefID: str=0001.0A020204.56CBCC29.0168, ss=1, re=0.000, fgs=0 Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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@bell.net 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;