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: 3238691 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2EE51C045B for ; Tue, 26 Nov 2013 10:41:13 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AD1E020383 for ; Tue, 26 Nov 2013 10:41:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7F6D72037B for ; Tue, 26 Nov 2013 10:41:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756017Ab3KZKkt (ORCPT ); Tue, 26 Nov 2013 05:40:49 -0500 Received: from sabertooth01.qualcomm.com ([65.197.215.72]:24995 "EHLO sabertooth01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754991Ab3KZKkn (ORCPT ); Tue, 26 Nov 2013 05:40:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qca.qualcomm.com; i=@qca.qualcomm.com; q=dns/txt; s=qcdkim; t=1385462443; x=1416998443; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=pRd/4xouB9wsmBCL+VLIidx3QGJNUtNlgxJeZt3a7b0=; b=sEicQClpgjBzpcO4D+zboXhvE4+Qg50Fj3dqXu/RlEZqq1djFDH/FReR vcfcbxYNlZG/IlPBfv123348plwCje/g2mtsBlM88aWzQgSxbiEgHiUPY shdWffy/qR/wI/Ksvbszmk/tqyjZlJ4IGG8fEpKXc8mBApfxLcvbhjCyK M=; X-IronPort-AV: E=McAfee;i="5400,1158,7270"; a="55793472" Received: from ironmsg04-l.qualcomm.com ([172.30.48.19]) by sabertooth01.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 CC: , , , , , 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] Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,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)