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 X-Spam-Level: X-Spam-Status: No, score=-0.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D93BC43215 for ; Mon, 2 Dec 2019 12:31:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D4071205ED for ; Mon, 2 Dec 2019 12:31:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hmC4OqRZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727459AbfLBMbt (ORCPT ); Mon, 2 Dec 2019 07:31:49 -0500 Received: from mail-ot1-f68.google.com ([209.85.210.68]:34366 "EHLO mail-ot1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727382AbfLBMbt (ORCPT ); Mon, 2 Dec 2019 07:31:49 -0500 Received: by mail-ot1-f68.google.com with SMTP id a15so3417949otf.1; Mon, 02 Dec 2019 04:31:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y3/WupH4Qim+Jtfwvw0/dF9jXvDuuoGX3pS6ZZoAhNc=; b=hmC4OqRZ+m+B5/FRgP/NMzRMPu4Mjh7PYtMnrrmI1qYtQTH2MK8Zcd2Wbouq1p7THb xgEiLOQaSRHebooCs1CnxEK3jH/hphrMh+PH5MCcXHsiTsNeoIu/h3q+pt1ci0vpXIfL vpqpGGw5xSEizZKCzoyOlkDdT14+8Kaz1MFp3dU+AiHpdzt8AwquVfhJgT6BzGn6+PcV w4kK+3yqnnBhP2JDXWx/B14bttTGZiuUjAPhaLg6sD4sUWyKLc0SblXRYGENSt2CYvWC HHBp8FrHo1JR4rJGYCvXgKiOZMj5vbDHIQgOR3sOrH04D5z5G6U+dAlvv0LnWAH3IdfG SITw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=y3/WupH4Qim+Jtfwvw0/dF9jXvDuuoGX3pS6ZZoAhNc=; b=h9CUL9844PBA53fm7rq9eQoohFKdUJ0MJBWacVnymiJGxWB/bAEedgRPW1sq9/fM5N 7K0ATC5IexwBCIeB7dbrbRTU8dlAu7WNb/wX2+FOI3d7x2zLRkzG7EpV5NcS9/oWNhiv m0Kfj0GpUFVRQQgN2BMY0djJciTv9GSy3ga4HPLdfZlw2l5ZmIBHqjGHzZuPvEFgehgt kucLFGdPMGT36YthCkcgFUfIYDZYOoi9k12xVQCwrkrU+eIaal0eiwyQ8tfbhK5pCVrP RpsUZHANHQG6YAvfVK6CygQbg5VxlsP55dcyO6fT6dLP5+hLWxN4ESkte8Q/YhTz+xsH 2Waw== X-Gm-Message-State: APjAAAVK3GpNhLCtILRr/KKXWfU6YZnsiB6d/WDAXlNlb9Mlg+MdiLtG U+9K5qsZMPL0ZrmDVr4MddFflgt756MRPnAzOXM= X-Google-Smtp-Source: APXvYqxTm0A+XWPhMBVpIwSs2hPG256uQnaCVIMOXQrA/Xdlgh+WtKadWFIU2w8KQ9lXr84fmeAEfBzAssk9fnm8xZU= X-Received: by 2002:a9d:3a37:: with SMTP id j52mr496572otc.39.1575289908085; Mon, 02 Dec 2019 04:31:48 -0800 (PST) MIME-Version: 1.0 References: <1575021070-28873-1-git-send-email-magnus.karlsson@intel.com> In-Reply-To: From: Magnus Karlsson Date: Mon, 2 Dec 2019 13:31:37 +0100 Message-ID: Subject: Re: [PATCH bpf] xsk: add missing memory barrier in xskq_has_addrs() To: Maxim Mikityanskiy Cc: Magnus Karlsson , "bjorn.topel@intel.com" , "ast@kernel.org" , "daniel@iogearbox.net" , "netdev@vger.kernel.org" , "jonathan.lemon@gmail.com" , "bpf@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Dec 2, 2019 at 10:30 AM Maxim Mikityanskiy wrote: > > On 2019-11-29 11:51, Magnus Karlsson wrote: > > The rings in AF_XDP between user space and kernel space have the > > following semantics: > > > > producer consumer > > > > if (LOAD ->consumer) { LOAD ->producer > > (A) smp_rmb() (C) > > STORE $data LOAD $data > > smp_wmb() (B) smp_mb() (D) > > STORE ->producer STORE ->consumer > > } > > > > The consumer function xskq_has_addrs() below loads the producer > > pointer and updates the locally cached copy of it. However, it does > > not issue the smp_rmb() operation required by the lockless ring. This > > would have been ok had the function not updated the locally cached > > copy, as that could not have resulted in new data being read from the > > ring. But as it updates the local producer pointer, a subsequent peek > > operation, such as xskq_peek_addr(), might load data from the ring > > without issuing the required smp_rmb() memory barrier. > > Thanks for paying attention to it, but I don't think it can really > happen. xskq_has_addrs only updates prod_tail, but xskq_peek_addr > doesn't use prod_tail, it reads from cons_tail to cons_head, and every > cons_head update has the necessary smp_rmb. You are correct, it cannot happen. I am working on a 10 part patch set that simplifies the rings and was staring blindly at that. In that patch set it can happen since I only have two cached pointers instead of four so there is a dependency, but not in the current code. I will include this barrier in my patch set at the appropriate place. Thanks for looking into this Maxim. Please drop this patch. /Magnus