From patchwork Thu Jun 17 14:05:59 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: FUJITA Tomonori X-Patchwork-Id: 56049 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id BA52EB7D5E for ; Fri, 18 Jun 2010 00:07:06 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760113Ab0FQOGa (ORCPT ); Thu, 17 Jun 2010 10:06:30 -0400 Received: from sh.osrg.net ([192.16.179.4]:46491 "EHLO sh.osrg.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760107Ab0FQOG1 (ORCPT ); Thu, 17 Jun 2010 10:06:27 -0400 Received: from localhost (rose.osrg.net [10.76.0.1]) by sh.osrg.net (8.14.3/8.14.3/OSRG-NET) with ESMTP id o5HE5xPS016333; Thu, 17 Jun 2010 23:05:59 +0900 Date: Thu, 17 Jun 2010 23:05:59 +0900 To: mchan@broadcom.com Cc: JBottomley@Novell.com, fujita.tomonori@lab.ntt.co.jp, vapier@gentoo.org, netdev@vger.kernel.org, linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: bnx2 fails to compile on parisc because of missing get_dma_ops() From: FUJITA Tomonori In-Reply-To: References: <1276780336.2789.6.camel@mulgrave.site> Mime-Version: 1.0 Message-Id: <20100617230435J.fujita.tomonori@lab.ntt.co.jp> Lines: 75 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-3.0 (sh.osrg.net [192.16.179.4]); Thu, 17 Jun 2010 23:06:00 +0900 (JST) X-Virus-Scanned: clamav-milter 0.96.1 at sh X-Virus-Status: Clean Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, 17 Jun 2010 06:30:39 -0700 "Michael Chan" wrote: > James Bottomley wrote: > > > On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote: > > > This prefetch improves performance noticeably when the driver is > > > handling incoming 64-byte packets at a sustained rate. > > > > So why not do it unconditionally? The worst that can happen > > is that you > > pull in a stale cache line which will get cleaned in the > > dma_sync, thus > > slightly degrading performance on incoherent architectures. > > The original patch was an unconditional prefetch. There was > some discussion that it might not be correct if the DMA wasn't > sync'ed yet on some archs. If the concensus is that it is ok to > do so, that would be the simplest solution. As James said, it just adds useless prefetch on incoherent architectures. sync_single_for_cpu is called later so we can see the correct data. One useless prefetch is unlikely to lead performance drop. You might prefer this v2. = From: FUJITA Tomonori Subject: [PATCH v2] bnx2: fix dma_get_ops compilation breakage This removes dma_get_ops() prefetch optimization in bnx2. bnx2 uses dma_get_ops() to see if dma_sync_single_for_cpu() is noop. bnx2 does prefetch if it's noop. But dma_get_ops() isn't available on all the architectures (only the architectures that uses dma_map_ops struct have it). Using dma_get_ops() in drivers leads to compilation breakage on many archtectures. This patch removes dma_get_ops() and changes bnx2 to do prefetch on all the architectures. This adds useless prefetch on incoherent architectures but this is harmless. It is also unlikely to cause the performance drop. Signed-off-by: FUJITA Tomonori Acked-by: Michael Chan --- drivers/net/bnx2.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index 949d7a9..85f1692 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -3099,12 +3099,10 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) skb = rx_buf->skb; prefetchw(skb); - if (!get_dma_ops(&pdev->dev)->sync_single_for_cpu) { - next_rx_buf = - &rxr->rx_buf_ring[ - RX_RING_IDX(NEXT_RX_BD(sw_cons))]; - prefetch(next_rx_buf->desc); - } + next_rx_buf = + &rxr->rx_buf_ring[RX_RING_IDX(NEXT_RX_BD(sw_cons))]; + prefetch(next_rx_buf->desc); + rx_buf->skb = NULL; dma_addr = dma_unmap_addr(rx_buf, mapping);