From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFF60C433EF for ; Mon, 13 Jun 2022 13:46:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1379005AbiFMNqr (ORCPT ); Mon, 13 Jun 2022 09:46:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1379227AbiFMNkE (ORCPT ); Mon, 13 Jun 2022 09:40:04 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF581D118; Mon, 13 Jun 2022 04:30:46 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7D1F561046; Mon, 13 Jun 2022 11:30:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8B429C34114; Mon, 13 Jun 2022 11:30:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1655119845; bh=Yq1pMyvdBvXCOYk3h9Hc/tgaOA+v+X0XmzrEY8AW2M4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DOM6AMWhTRShuABD3RG2HE0BNDndTD0GuTi9hO8eUNUnhZVs0ttMlJ8SPrQ0gOwIa tufYhoW1wtLTzEAdngdZV6FK5xCxCaakLHZRvtAEZP+2Vgs1KdsCBGk31gqrDvwECw Idi35oPgooy6C7kcTzZc3Z/7CsxwXV2yI72HSbJs= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Guoju Fang , Jakub Kicinski , Sasha Levin Subject: [PATCH 5.18 130/339] net: sched: add barrier to fix packet stuck problem for lockless qdisc Date: Mon, 13 Jun 2022 12:09:15 +0200 Message-Id: <20220613094930.453486060@linuxfoundation.org> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220613094926.497929857@linuxfoundation.org> References: <20220613094926.497929857@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Guoju Fang [ Upstream commit 2e8728c955ce0624b958eee6e030a37aca3a5d86 ] In qdisc_run_end(), the spin_unlock() only has store-release semantic, which guarantees all earlier memory access are visible before it. But the subsequent test_bit() has no barrier semantics so may be reordered ahead of the spin_unlock(). The store-load reordering may cause a packet stuck problem. The concurrent operations can be described as below, CPU 0 | CPU 1 qdisc_run_end() | qdisc_run_begin() . | . ----> /* may be reorderd here */ | . | . | . | spin_unlock() | set_bit() | . | smp_mb__after_atomic() ---- test_bit() | spin_trylock() . | . Consider the following sequence of events: CPU 0 reorder test_bit() ahead and see MISSED = 0 CPU 1 calls set_bit() CPU 1 calls spin_trylock() and return fail CPU 0 executes spin_unlock() At the end of the sequence, CPU 0 calls spin_unlock() and does nothing because it see MISSED = 0. The skb on CPU 1 has beed enqueued but no one take it, until the next cpu pushing to the qdisc (if ever ...) will notice and dequeue it. This patch fix this by adding one explicit barrier. As spin_unlock() and test_bit() ordering is a store-load ordering, a full memory barrier smp_mb() is needed here. Fixes: a90c57f2cedd ("net: sched: fix packet stuck problem for lockless qdisc") Signed-off-by: Guoju Fang Link: https://lore.kernel.org/r/20220528101628.120193-1-gjfang@linux.alibaba.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- include/net/sch_generic.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 80973ce820f3..d6cf5116b5f9 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -209,6 +209,12 @@ static inline void qdisc_run_end(struct Qdisc *qdisc) if (qdisc->flags & TCQ_F_NOLOCK) { spin_unlock(&qdisc->seqlock); + /* spin_unlock() only has store-release semantic. The unlock + * and test_bit() ordering is a store-load ordering, so a full + * memory barrier is needed here. + */ + smp_mb(); + if (unlikely(test_bit(__QDISC_STATE_MISSED, &qdisc->state))) __netif_schedule(qdisc); -- 2.35.1