From patchwork Tue Nov 26 10:40:19 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalle Valo X-Patchwork-Id: 3238701 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D5ED49F3A0 for ; Tue, 26 Nov 2013 10:41:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9FB9720398 for ; Tue, 26 Nov 2013 10:41:28 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BB0102037B for ; Tue, 26 Nov 2013 10:41:23 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VlG4z-0002xi-59; Tue, 26 Nov 2013 10:41:17 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VlG4w-0008Io-IT; Tue, 26 Nov 2013 10:41:14 +0000 Received: from sabertooth02.qualcomm.com ([65.197.215.38]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VlG4t-0008Ho-H2 for linux-arm-kernel@lists.infradead.org; Tue, 26 Nov 2013 10:41:12 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qca.qualcomm.com; i=@qca.qualcomm.com; q=dns/txt; s=qcdkim; t=1385462471; x=1416998471; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=pRd/4xouB9wsmBCL+VLIidx3QGJNUtNlgxJeZt3a7b0=; b=iREQlQ9kfJw/DpOMVM1Bx89BPPlC8Hl5j9iPqLNVSL16zGdTcjDSpVgf x8jupPjJ9CmXMnOJXFJABPSVMJUX412SH0rgvN05auiteZ8waCEpQf/H6 znTWK92VYrAJ7y1yaBCizVMC8hnEssS3bwuI/+y+cqMadTVizZRnybVL2 Q=; X-IronPort-AV: E=McAfee;i="5400,1158,7270"; a="55892389" Received: from ironmsg04-l.qualcomm.com ([172.30.48.19]) by sabertooth02.qualcomm.com with ESMTP; 26 Nov 2013 02:40:43 -0800 X-IronPort-AV: E=McAfee;i="5400,1158,7270"; a="553052890" Received: from nasanexhc04.na.qualcomm.com ([172.30.48.17]) by Ironmsg04-L.qualcomm.com with ESMTP/TLS/RC4-SHA; 26 Nov 2013 02:40:25 -0800 Received: from potku.adurom.net.qca.qualcomm.com (172.30.48.1) by qcmail1.qualcomm.com (172.30.48.17) with Microsoft SMTP Server (TLS) id 14.3.158.1; Tue, 26 Nov 2013 02:40:24 -0800 From: Kalle Valo To: Jason Liu Subject: Re: [PATCH] ath6kl: sdio: fix system panic when doing wifi stress test References: <1384221185-8645-1-git-send-email-r64343@freescale.com> Date: Tue, 26 Nov 2013 12:40:19 +0200 In-Reply-To: <1384221185-8645-1-git-send-email-r64343@freescale.com> (Jason Liu's message of "Tue, 12 Nov 2013 09:53:05 +0800") Message-ID: <87k3fvjvjg.fsf@kamboji.qca.qualcomm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 X-Originating-IP: [172.30.48.1] X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131126_054111_745449_E50EA2B7 X-CRM114-Status: GOOD ( 23.55 ) X-Spam-Score: -2.0 (--) Cc: netdev@vger.kernel.org, ath6kl-devel@qca.qualcomm.com, linux-wireless@vger.kernel.org, linville@tuxdriver.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable 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 Hi Jason, Jason Liu writes: > When did the wifi iperf test, meet one following kernel panic: > command: iperf -c $TARGET_IP -i 5 -t 50 -w 1M > > Unable to handle kernel paging request at virtual address 1a480000 > pgd = 80004000 > [1a480000] *pgd=00000000 > Internal error: Oops: 805 [#1] SMP ARM [...] > The kernel panic is caused by the sg_buf is not set correctly with the > following code when compiled with Yocto GCC 4.8.1: > > drivers/net/wireless/ath/ath6kl/hif.h: > struct hif_scatter_req { > struct list_head list; > /* address for the read/write operation */ > u32 addr; > ... > > /* bounce buffer for upper layers to copy to/from */ > u8 *virt_dma_buf; > > struct hif_scatter_item scat_list[1]; > > u32 scat_q_depth; > }; > > (Note: the scat_req.scat_list[] will dynamiclly grow with run-time) There's actually a major bug right there, scat_list can corrupt scat_q_depth. > The GCC 4.8.1 compiler will not do the for-loop till scat_entries, instead, > it only run one round loop. This may be caused by that the GCC 4.8.1 thought > that the scat_list only have one item and then no need to do full iteration, > but this is simply wrong by looking at the assebly code. This will cause the > sg buffer not get set when scat_entries > 1 and thus lead to kernel panic. > > This patch is a workaround to the GCC 4.8.1 complier issue by passing the > entry address of the scat_req->scat_list to the for-loop and interate it, > then, GCC 4.8.1 will do the full for-loop correctly. > (Note: This issue not observed with GCC 4.7.2, only found on the GCC 4.8.1) > > This patch does not change any function logic and no any performance downgrade. [...] > + scat_list = &scat_req->scat_list[0]; > + > /* assemble SG list */ > - for (i = 0; i < scat_req->scat_entries; i++, sg++) { > + for (i = 0; i < scat_req->scat_entries; i++, sg++, scat_list++) { > ath6kl_dbg(ATH6KL_DBG_SCATTER, "%d: addr:0x%p, len:%d\n", > - i, scat_req->scat_list[i].buf, > - scat_req->scat_list[i].len); > + i, scat_list->buf, scat_list->len); > > - sg_set_buf(sg, scat_req->scat_list[i].buf, > - scat_req->scat_list[i].len); > + sg_set_buf(sg, scat_list->buf, scat_list->len); > } Working around the problem by adding a temporary variable makes me a bit worried, I would rather fix the root cause. Is the root cause by that we define the field with scat_list[1]? Does the patch below help? It would also fix the corruption with scat_q_depth. Please note that I have only compile tested it. And I might have also missed something important, so please review it carefully. Acked-by: Jason Liu Tested-by: Jason Liu --- a/drivers/net/wireless/ath/ath6kl/hif.h +++ b/drivers/net/wireless/ath/ath6kl/hif.h @@ -197,9 +197,9 @@ struct hif_scatter_req { /* bounce buffer for upper layers to copy to/from */ u8 *virt_dma_buf; - struct hif_scatter_item scat_list[1]; - u32 scat_q_depth; + + struct hif_scatter_item scat_list[0]; }; struct ath6kl_irq_proc_registers { diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c index 7126bdd..6bf15a3 100644 --- a/drivers/net/wireless/ath/ath6kl/sdio.c +++ b/drivers/net/wireless/ath/ath6kl/sdio.c @@ -348,7 +348,7 @@ static int ath6kl_sdio_alloc_prep_scat_req(struct ath6kl_sdio *ar_sdio, int i, scat_req_sz, scat_list_sz, size; u8 *virt_buf; - scat_list_sz = (n_scat_entry - 1) * sizeof(struct hif_scatter_item); + scat_list_sz = n_scat_entry * sizeof(struct hif_scatter_item); scat_req_sz = sizeof(*s_req) + scat_list_sz; if (!virt_scat)