From patchwork Sat Apr 30 14:01:14 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ivo van Doorn X-Patchwork-Id: 743512 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p3UE1cM4015912 for ; Sat, 30 Apr 2011 14:01:38 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751992Ab1D3OB2 (ORCPT ); Sat, 30 Apr 2011 10:01:28 -0400 Received: from mail-ey0-f174.google.com ([209.85.215.174]:37439 "EHLO mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754622Ab1D3OBV (ORCPT ); Sat, 30 Apr 2011 10:01:21 -0400 Received: by eyx24 with SMTP id 24so1380044eyx.19 for ; Sat, 30 Apr 2011 07:01:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:from:to:subject:date:user-agent:cc:references :in-reply-to:mime-version:content-type:content-transfer-encoding :message-id; bh=3u6xkrjGBnjYWv+5+h5RSfrg8eDgkwQ+25za0HEEVck=; b=ZcML5DyAXKfmphXGSP9vGrCDxmvIT1I3dzQ6WKICrW8vy1zVhtCdRHpUD0GscIPx77 189PG3xB1J6qPHwWGM2jY6MUw/1hxxOBo8foszpr51P+S+3uOztocOqx4/YX4mQlM5Wt Ef7t/Jhh8SaLumECAfugL6bmauegsqem0UjHk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:subject:date:user-agent:cc:references:in-reply-to :mime-version:content-type:content-transfer-encoding:message-id; b=SJ9jBcVLw78Fdhv/OQ2o8dcqhW63r7862oJ0gfKCRY7i2EXf/JdQWKdI2YHHNyJG/k OXsfF1WOgZ/mTJx9TOt/UlQszmqEK/35dbqGz5HDc8WNoWEgREJLVCMJBaMAA9qXNgJo 6mPRip1kuYXwO8Dck9rCLsxShgff4RjfAZp6s= Received: by 10.213.9.12 with SMTP id j12mr2607581ebj.147.1304172079808; Sat, 30 Apr 2011 07:01:19 -0700 (PDT) Received: from localhost.localdomain (g121037.upc-g.chello.nl [80.57.121.37]) by mx.google.com with ESMTPS id s50sm2764501eeh.1.2011.04.30.07.01.17 (version=SSLv3 cipher=OTHER); Sat, 30 Apr 2011 07:01:18 -0700 (PDT) From: Ivo van Doorn To: Gertjan van Wingerde Subject: Re: [PATCH 04/23] rt2x00: Make rt2x00_queue_entry_for_each more flexible Date: Sat, 30 Apr 2011 16:01:14 +0200 User-Agent: KMail/1.13.5 (Linux/2.6.32.26-175.fc12.x86_64; KDE/4.4.5; x86_64; ; ) Cc: Yasushi SHOJI , helmut.schaa@googlemail.com, hanada@atmark-techno.com, linux-wireless@vger.kernel.org References: <201104181526.01722.IvDoorn@gmail.com> <4DBA556E.6090602@gmail.com> In-Reply-To: <4DBA556E.6090602@gmail.com> MIME-Version: 1.0 Message-Id: <201104301601.15558.IvDoorn@gmail.com> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 30 Apr 2011 14:01:38 +0000 (UTC) Hi, > >> My colleague just tested this patch set and found out that the in > >> question makes our 400MHz ARM cpu board with ralink usb dongle non > >> functional due to high cpu consumption. it seems for us that exiting > >> from function every time it finds an entry is too expensive on systems > >> slower than PCs. > > > > Interesting, I would suspected the patch to reduce the CPU consumption > > rather then increasing it. I can do some testing regarding this looping > > during the weekend, but I haven't seen high CPU consumption on my > > system during my last test. However I wasn't testing on an embedded system... > > > >> To verify our thought, we changed the source code as the patch below. > >> What we intended to do with this change is to continue processing all > >> entry without breaking semantics. > >> > >> With the patch below our board seem to work fine again, but not sure > >> exactly why it takes so much time to check the list again. We are not > >> against the idea of the patch at all. We just want to ask you guys > >> how we should go to track this problem. it might be the slow usb? > > > > Could you try use debugfs and see if some queue and packet counters > > of mac80211/rt2x00 show excessive values? > > > > Have you been running the test with powersaving enabled or disabled? > > > > Yeah, I had the same impression as Ivo, that it should save CPU cycles, > especially since the patch was submitted by Helmut, who is very keen > on saving CPU cycles, as he tests on an embedded (PCI-based) platform as > well. > > But, if this proves to not be useful for now, then we should revert the > entire patch, as the patch proposed simply negates the intended behavior > and leaves a mess. > > I believe the offending patch was preparation for another patch which so > far has not materialized in a usable form, so there should be no harm in > reverting the offending patch. I've have a different idea, after a discussion with Helmut, I had changed his original patch. But I think below patch should resolve the issue as well. Yasushi, can you please test this patch. Thanks, Ivo --- -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c index bfbb446..6b0ca7e 100644 --- a/drivers/net/wireless/rt2x00/rt2x00usb.c +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c @@ -392,7 +392,7 @@ static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry, void* data) if (test_and_set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) || test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags)) - return true; + return false; rt2x00lib_dmastart(entry); @@ -447,7 +447,7 @@ static bool rt2x00usb_flush_entry(struct queue_entry *entry, void* data) struct queue_entry_priv_usb_bcn *bcn_priv = entry->priv_data; if (!test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)) - return true; + return false; usb_kill_urb(entry_priv->urb);