From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-f193.google.com (mail-pg1-f193.google.com [209.85.215.193]) by dpdk.org (Postfix) with ESMTP id 163151B1DA for ; Mon, 8 Oct 2018 23:53:19 +0200 (CEST) Received: by mail-pg1-f193.google.com with SMTP id 23-v6so8405566pgc.8 for ; Mon, 08 Oct 2018 14:53:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=8mS/H+AdVItMbxECm93FKBxD4KYV/78CJmjs89w+PjE=; b=Yf0jWRamK4WFKYneb5SN+bJ+o6VFD92EryBvYaBwPuQEs2KnsyMgAh26aKc1+zyDj2 s3gRUppb+4LajOIYLfPD3VaUtrkH5SAfwESZ5PWrksnzqqfrU4evdgWH5qoS0kZ9NlUc JjmTlsJyXuuaD3OGzirZQ0Q8ZgrjvSIzRa+9VgiYGsNM2qYW+ukFjmgZeEZHNqcPCCHD tVlFtNzz4QD0xm9QEBKvSBGe8O7y/O6FbvfvAgLl3/VFE+JbBY260h/7r7Q6bgm73g0k D/h5rGoZSRGZT06/ih470EjTNO7fFNxvKgd0EH/0HrHiLGKXEWX4Vk78ahB7B/zelFmL ndGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=8mS/H+AdVItMbxECm93FKBxD4KYV/78CJmjs89w+PjE=; b=IjIJffTsrJnJHKxlNDHBFg/Sbk+oP1Igi2cNbA/j/Ag5VYAGRGVadUnuFVXZsqM1kN v3gxYOMBod6RGrSLe/ppR8xjlyoaphdr6w7RfaDydd/kz6OtZ2zj5KoIUXW4wiDBpGm7 47vz31O/RNFOQ37IoUrcCHei80YCOBVXbhntWklevC2MwG/K7E99uy7OPwJmnhDcacjt 5WJg0mXzG8PtmXtpdeJ3evJHlqNW5WED1/XDRg8j2QVvswjTakLC1UqcPYgR9t2TyWe8 fOvzN+K6E0SbVIo4w73RGaLRwv+urfHHEAdJJmW8+yoK+7oyCDLsYbnpR9Z0ObHVEZO5 UTxQ== X-Gm-Message-State: ABuFfojjqHC4H5DxT6w7QJt1BN4ww4JjSjoutxn2RN2ZUCCUpXuki3+0 3OijG4RNsiltGkKVwa9ha06qZw== X-Google-Smtp-Source: ACcGV63Hv8ac04iwhZ4rhJyZIeTmXD/C/JfXx/yv9rI8f3DUbrCHSgn0buUTyLblsX6wF56sh1Ro/w== X-Received: by 2002:a62:2542:: with SMTP id l63-v6mr21089086pfl.64.1539035599077; Mon, 08 Oct 2018 14:53:19 -0700 (PDT) Received: from xeon-e3 (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id j64-v6sm9137069pge.15.2018.10.08.14.53.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 08 Oct 2018 14:53:18 -0700 (PDT) Date: Mon, 8 Oct 2018 14:53:08 -0700 From: Stephen Hemminger To: Phil Yang Cc: dev@dpdk.org, jerin.jacob@caviumnetworks.com, Gavin.Hu@arm.com, Honnappa.Nagarahalli@arm.com, Ola.Liljedahl@arm.com, ferruh.yigit@intel.com Message-ID: <20181008145308.0fbc64de@xeon-e3> In-Reply-To: <1538989906-8349-2-git-send-email-phil.yang@arm.com> References: <1537364560-4124-1-git-send-email-phil.yang@arm.com> <1538989906-8349-1-git-send-email-phil.yang@arm.com> <1538989906-8349-2-git-send-email-phil.yang@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo synchronization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Oct 2018 21:53:20 -0000 On Mon, 8 Oct 2018 17:11:44 +0800 Phil Yang wrote: > diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h > index ac26a8c..70ac14e 100644 > --- a/lib/librte_kni/rte_kni_fifo.h > +++ b/lib/librte_kni/rte_kni_fifo.h > @@ -28,8 +28,9 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num) > { > unsigned i = 0; > unsigned fifo_write = fifo->write; > - unsigned fifo_read = fifo->read; > unsigned new_write = fifo_write; > + rte_smp_rmb(); > + unsigned fifo_read = fifo->read; > The patch makes sense, but this function should be changed to match kernel code style. That means no declarations after initial block, and use 'unsigned int' rather than 'unsigned' Also. why is i initialized? Best practice now is to not do gratitious initialization since it defeats compiler checks for accidental use of uninitialized variables. What makes sense is something like: kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num) { unsigned int i, fifo_read, fifo_write, new_write; fifo_write = fifo->write; new_write = fifo_write; rte_smb_rmb(); fifo_read = fifo->read; Sorry, blaming you for issues which are inherited from original KNI code. Maybe someone should run kernel checkpatch (not DPDK checkpatch) on it and fix those.