From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-x22d.google.com (mail-wi0-x22d.google.com [IPv6:2a00:1450:400c:c05::22d]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id C562921F1F1; Wed, 16 Apr 2014 10:00:36 -0700 (PDT) Received: by mail-wi0-f173.google.com with SMTP id z2so1720206wiv.6 for ; Wed, 16 Apr 2014 10:00:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=Fv1cU2hOlNtXc4vhsvlRGN94QH0B1cMZ8YOGoeM0ktE=; b=lr/TXWLw6lohdwebAs50TOIXlEsmymN7NeeI4B/79yuWSJ120cfiYWq0fLFuvvnFy8 +fPUY1BTCtd/50PtGGLO3rLUkY4Tm51k7yfrTQJXEfK1yG87TOW+Bpp3wGoMAIOFIe4C mcsmbXnoxuJqqKYfbDOoqx85FlECEMECxSm5Ygu7JYBT13MQPIvKrFphuimz9R/IxA3N EfrZRM3cNxgLHGvRlhDFYAK2C/LCi4W58uE7p/S0kQeZpoGUO5uRe3LSS4oQV9aNXz89 Uqt6t8v4CGfYaRBo9Onqnc0UgVQ345rOs4hDVN0RgS0N2Rz0k+tgkTPMinkD6ZjFpjHv znhQ== MIME-Version: 1.0 X-Received: by 10.180.37.178 with SMTP id z18mr20397542wij.46.1397667634714; Wed, 16 Apr 2014 10:00:34 -0700 (PDT) Received: by 10.216.177.10 with HTTP; Wed, 16 Apr 2014 10:00:34 -0700 (PDT) In-Reply-To: <534EB61C.1080108@openwrt.org> References: <534E8168.50003@openwrt.org> <534EB61C.1080108@openwrt.org> Date: Wed, 16 Apr 2014 10:00:34 -0700 Message-ID: From: Dave Taht To: Felix Fietkau Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: cerowrt@lists.bufferbloat.net, "cerowrt-devel@lists.bufferbloat.net" Subject: Re: [Cerowrt-devel] Fwd: [bug #442] ath9k queue hang X-BeenThere: cerowrt-devel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: Development issues regarding the cerowrt test router project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Apr 2014 17:00:37 -0000 should I have said "de-protected"? in linux-3.14/drivers/net/wireless/ath/ath9k/xmit.c ath_txq_lock(sc, txq); if (txq =3D=3D sc->tx.txq_map[q] && ++txq->pending_frames > sc->tx.txq_max_pending[q] && !txq->stopped) { ieee80211_stop_queue(sc->hw, q); txq->stopped =3D true; } if (txctl->an && ieee80211_is_data_present(hdr->frame_control)) tid =3D ath_get_skb_tid(sc, txctl->an, skb); if (info->flags & IEEE80211_TX_CTL_PS_RESPONSE) { ath_txq_unlock(sc, txq); txq =3D sc->tx.uapsdq; ^^^^^^ ath_txq_lock(sc, txq); } else if (txctl->an && On Wed, Apr 16, 2014 at 9:55 AM, Felix Fietkau wrote: > On 2014-04-16 17:34, Dave Taht wrote: >> On Wed, Apr 16, 2014 at 6:11 AM, Felix Fietkau wrote: >>> On 2014-04-15 21:00, Dave Taht wrote: >>>> Thx felix! >>>> >>>> Given that there seems to be a potential race in the code >>>> review I did at: >>>> >>>> http://www.bufferbloat.net/issues/442#note-22 >>>> >>>> another thought is to make the increment and decrement of >>>> >>>> txq->pending_frame atomic, or to do a flush before the unlock >>> I'm not convinced that there's a race that involves txq->pending_frames= . >>> There is no need to make the increment/decrement atomic, because that >>> variable is already protected by the txq lock. >> >> It and "stopped" are briefly unprotected along that code path. > Where? > > - Felix --=20 Dave T=C3=A4ht NSFW: https://w2.eff.org/Censorship/Internet_censorship_bills/russell_0296_= indecent.article