From patchwork Sun Aug 23 15:30:37 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Krzysztof Halasa X-Patchwork-Id: 31877 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 17C0EB7B61 for ; Mon, 24 Aug 2009 01:30:59 +1000 (EST) Received: by ozlabs.org (Postfix) id 04069DDD0B; Mon, 24 Aug 2009 01:30:59 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 88D80DDD04 for ; Mon, 24 Aug 2009 01:30:58 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933855AbZHWPak (ORCPT ); Sun, 23 Aug 2009 11:30:40 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S933849AbZHWPaj (ORCPT ); Sun, 23 Aug 2009 11:30:39 -0400 Received: from khc.piap.pl ([195.187.100.11]:48503 "EHLO khc.piap.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933839AbZHWPai (ORCPT ); Sun, 23 Aug 2009 11:30:38 -0400 Received: from intrepid.localdomain (intrepid.localdomain [10.0.0.2]) by khc.piap.pl (Postfix) with ESMTP id 0A077931F; Sun, 23 Aug 2009 17:30:37 +0200 (CEST) To: David Miller , Walt Holman Cc: netdev@vger.kernel.org, jeffrey.t.kirsher@intel.com, jesse.brandeburg@intel.com, bruce.w.allan@intel.com, peter.p.waskiewicz.jr@intel.com, john.ronciak@intel.com Subject: Re: E100 RX ring buffers continued... References: <20090822.181552.152398363.davem@davemloft.net> From: Krzysztof Halasa Date: Sun, 23 Aug 2009 17:30:37 +0200 In-Reply-To: <20090822.181552.152398363.davem@davemloft.net> (David Miller's message of "Sat\, 22 Aug 2009 18\:15\:52 -0700 \(PDT\)") Message-ID: MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org David Miller writes: > I think going down the road of trying to use the flexible mode is a > dead end. I doubt any other OS driver is using it, and that means > that there are likely many other errata hiding in the bushes which you > will unearth by trying to use this new descriptor mode. And it won't > show up when you test it, it will show up when some random person in > some remote data center somewhere updates their kernel, and they won't > send us a bug report, they'll replace their card or downgrade their > kernel instead. Well, I'm afraid it's a possible scenario. I won't touch the flexible mode unless Intel folks tell me it's safe. OTOH it seems it was used by less common software, at least in the 82557-9 times. Not sure about Windows driver. (It seems the simplified mode was meant for Linux-alike "1 buffer per packet" approach while the flexible mode was to support systems using mbuf-like structures). > Just make the driver use consistent buffers for RX, and when packets > arrive an SKB is allocated and the packet data is copied into the SKB >>From the consistent buffer. That would be a performance hit, wouldn't it? Especially in older machines, where e100 was typically installed. I think it should be the last resort. > And for 2.6.31-rcX we probably have to simply revert your change. Unfortunately it seems that reverting will not fix operation completely on a system with swiotlb (checking the descriptor status isn't the only racy operation which depends on the cache behaviour, though it's the most frequent). And it will break all non-coherent archs again, they will either need to use the patch or still stick to (already removed) eepro100.c I looked at the eepro100.c sources. It uses BIDIR mapping the same way as e100.c does, but then syncs using FROM_DEVICE/TO_DEVICE instead of e100's always-BIDIR. I think the same in e100.c would work on all platforms (though still violating the DMA API a bit). Perhaps we should do that instead? Walt, can you check if 2.6.30.5 with the following patch applied still breaks e100 with the 6 GB of RAM, please? TIA. (This patch makes swiotlb aware that the CPU didn't alter the buffer, though I don't know if swiotlb will use this info). diff --git a/drivers/net/e100.c b/drivers/net/e100.c index 014dfb6..53e8252 100644 --- a/drivers/net/e100.c +++ b/drivers/net/e100.c @@ -1764,7 +1764,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx, nic->ru_running = RU_SUSPENDED; pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr, sizeof(struct rfd), - PCI_DMA_BIDIRECTIONAL); + PCI_DMA_FROMDEVICE); return -ENODATA; }