patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Gavin Hu <gavin.hu@arm.com>
Cc: dev@dpdk.org, Honnappa.Nagarahalli@arm.com, steve.capper@arm.com,
	Ola.Liljedahl@arm.com, nd@arm.com, stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v3 1/3] ring: read tail using atomic load
Date: Sat, 29 Sep 2018 16:18:59 +0530	[thread overview]
Message-ID: <20180929104857.GA30457@jerin> (raw)
In-Reply-To: <1537172244-64874-1-git-send-email-gavin.hu@arm.com>

-----Original Message-----
> Date: Mon, 17 Sep 2018 16:17:22 +0800
> From: Gavin Hu <gavin.hu@arm.com>
> To: dev@dpdk.org
> CC: gavin.hu@arm.com, Honnappa.Nagarahalli@arm.com, steve.capper@arm.com,
>  Ola.Liljedahl@arm.com, jerin.jacob@caviumnetworks.com, nd@arm.com,
>  stable@dpdk.org
> Subject: [PATCH v3 1/3] ring: read tail using atomic load
> X-Mailer: git-send-email 2.7.4
> 
> External Email
> 
> In update_tail, read ht->tail using __atomic_load.Although the
> compiler currently seems to be doing the right thing even without
> _atomic_load, we don't want to give the compiler freedom to optimise
> what should be an atomic load, it should not be arbitarily moved
> around.
> 
> Fixes: 39368ebfc6 ("ring: introduce C11 memory model barrier option")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <Ola.Liljedahl@arm.com>
> ---
>  lib/librte_ring/rte_ring_c11_mem.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
> index 94df3c4..234fea0 100644
> --- a/lib/librte_ring/rte_ring_c11_mem.h
> +++ b/lib/librte_ring/rte_ring_c11_mem.h
> @@ -21,7 +21,8 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
>          * we need to wait for them to complete
>          */
>         if (!single)
> -               while (unlikely(ht->tail != old_val))
> +               while (unlikely(old_val != __atomic_load_n(&ht->tail,
> +                                               __ATOMIC_RELAXED)))
>                         rte_pause();

Since it is a while loop with rte_pause(), IMO, There is no scope of false compiler optimization.
IMO, this change may not required though I don't see any performance
difference with two core ring_perf_autotest test. May be more core
case it may have effect. IMO, If it not absolutely required, we can avoid
this change.

> 
>         __atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
> --
> 2.7.4
> 

  parent reply	other threads:[~2018-09-29 10:49 UTC|newest]

Thread overview: 125+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-06  1:18 [dpdk-stable] [PATCH] ring: fix c11 memory ordering issue Gavin Hu
2018-08-06  9:19 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2018-08-08  1:39   ` Gavin Hu
2018-08-07  3:19 ` [dpdk-stable] [PATCH v2] " Gavin Hu
2018-08-07  5:56   ` He, Jia
2018-08-07  7:56     ` Gavin Hu
2018-08-08  3:07       ` Jerin Jacob
2018-08-08  7:23         ` Thomas Monjalon
     [not found]   ` <20180917074735.28161-1-gavin.hu@arm.com>
2018-09-17  7:47     ` [dpdk-stable] [PATCH v3 3/3] doc: add cross compile part for sample applications Gavin Hu
2018-09-17  9:48       ` Jerin Jacob
2018-09-17 10:28         ` Gavin Hu (Arm Technology China)
2018-09-17 10:34           ` Jerin Jacob
2018-09-17 10:55             ` Gavin Hu (Arm Technology China)
2018-09-17 10:49       ` [dpdk-stable] [PATCH v4] " Gavin Hu
2018-09-17 10:53         ` [dpdk-stable] [PATCH v5] " Gavin Hu
2018-09-18 11:00           ` Jerin Jacob
2018-09-19  0:33           ` [dpdk-stable] [PATCH v6] " Gavin Hu
2018-09-17  8:11     ` [dpdk-stable] [PATCH v4 1/4] bus/fslmc: fix undefined reference of memsegs Gavin Hu
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 2/4] ring: read tail using atomic load Gavin Hu
2018-09-20  6:41         ` Jerin Jacob
2018-09-25  9:26           ` Gavin Hu (Arm Technology China)
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 3/4] ring: synchronize the load and store of the tail Gavin Hu
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 4/4] ring: move the atomic load of head above the loop Gavin Hu
2018-10-27 14:21         ` Thomas Monjalon
2018-09-17  8:17   ` [dpdk-stable] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu
2018-09-17  8:17     ` [dpdk-stable] [PATCH v3 2/3] ring: synchronize the load and store of the tail Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26  9:59         ` Justin He
2018-09-29 10:57       ` Jerin Jacob
2018-10-17  6:29       ` [dpdk-stable] [PATCH 1/2] " Gavin Hu
2018-10-17  6:29         ` [dpdk-stable] [PATCH 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-17  6:35         ` [dpdk-stable] [PATCH 1/2] ring: synchronize the load and store of the tail Gavin Hu (Arm Technology China)
2018-10-27 14:39           ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2018-10-27 15:00             ` Jerin Jacob
2018-10-27 15:13               ` Thomas Monjalon
2018-10-27 15:34                 ` Jerin Jacob
2018-10-27 15:48                   ` Thomas Monjalon
2018-10-29  2:51                   ` Gavin Hu (Arm Technology China)
2018-10-29  2:57                   ` Gavin Hu (Arm Technology China)
2018-10-29 10:16                     ` Jerin Jacob
2018-10-29 10:47                       ` Thomas Monjalon
2018-10-29 11:10                         ` Jerin Jacob
2018-11-03 20:12                 ` Mattias Rönnblom
2018-11-05 21:51                   ` Honnappa Nagarahalli
2018-11-06 11:03                     ` Mattias Rönnblom
     [not found]         ` <1540955698-209159-1-git-send-email-gavin.hu@arm.com>
2018-10-31  3:14           ` [dpdk-stable] [PATCH v2 " Gavin Hu
2018-10-31  3:14           ` [dpdk-stable] [PATCH v2 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  3:35         ` [dpdk-stable] [PATCH v2 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31  3:35         ` [dpdk-stable] [PATCH v2 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  9:36           ` Thomas Monjalon
2018-10-31 10:27             ` Gavin Hu (Arm Technology China)
2018-09-17  8:17     ` [dpdk-stable] [PATCH v3 3/3] " Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26 10:06         ` Justin He
2018-09-29  7:19           ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2018-09-29 10:59       ` [dpdk-stable] " Jerin Jacob
2018-09-26  9:29     ` [dpdk-stable] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu (Arm Technology China)
2018-09-26 10:09       ` Justin He
2018-09-29 10:48     ` Jerin Jacob [this message]
2018-10-05  0:47       ` Gavin Hu (Arm Technology China)
2018-10-05  8:21         ` Ananyev, Konstantin
2018-10-05 11:15           ` Ola Liljedahl
2018-10-05 11:36             ` Ola Liljedahl
2018-10-05 13:44               ` Ananyev, Konstantin
2018-10-05 14:21                 ` Ola Liljedahl
2018-10-05 15:11                 ` Honnappa Nagarahalli
2018-10-05 17:07                   ` Jerin Jacob
2018-10-05 18:05                     ` Ola Liljedahl
2018-10-05 20:06                       ` Honnappa Nagarahalli
2018-10-05 20:17                         ` Ola Liljedahl
2018-10-05 20:29                           ` Honnappa Nagarahalli
2018-10-05 20:34                             ` Ola Liljedahl
2018-10-06  7:41                               ` Jerin Jacob
2018-10-06 19:44                                 ` Ola Liljedahl
2018-10-06 19:59                                   ` Ola Liljedahl
2018-10-07  4:02                                   ` Jerin Jacob
2018-10-07 20:11                                     ` Ola Liljedahl
2018-10-07 20:44                                     ` Ola Liljedahl
2018-10-08  6:06                                       ` Jerin Jacob
2018-10-08  9:22                                         ` Ola Liljedahl
2018-10-08 10:00                                           ` Jerin Jacob
2018-10-08 10:25                                             ` Ola Liljedahl
2018-10-08 10:33                                               ` Gavin Hu (Arm Technology China)
2018-10-08 10:39                                                 ` Ola Liljedahl
2018-10-08 10:41                                                   ` Gavin Hu (Arm Technology China)
2018-10-08 10:49                                                 ` Jerin Jacob
2018-10-10  6:28                                                   ` Gavin Hu (Arm Technology China)
2018-10-10 19:26                                                     ` Honnappa Nagarahalli
2018-10-08 10:46                                               ` Jerin Jacob
2018-10-08 11:21                                                 ` Ola Liljedahl
2018-10-08 11:50                                                   ` Jerin Jacob
2018-10-08 11:59                                                     ` Ola Liljedahl
2018-10-08 12:05                                                       ` Jerin Jacob
2018-10-08 12:20                                                         ` [dpdk-stable] [dpdk-dev] " Jerin Jacob
2018-10-08 12:30                                                           ` Ola Liljedahl
2018-10-09  8:53                                                             ` Olivier Matz
2018-10-09  3:16                                             ` [dpdk-stable] " Honnappa Nagarahalli
2018-10-08 14:43                                           ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
2018-10-08 14:46                                             ` Ola Liljedahl
2018-10-08 15:45                                               ` Ola Liljedahl
2018-10-08  5:27                               ` [dpdk-stable] " Honnappa Nagarahalli
2018-10-08 10:01                                 ` Ola Liljedahl
2018-10-27 14:17     ` Thomas Monjalon
     [not found] <1541157688-40012-1-git-send-email-gavin.hu@arm.com>
     [not found] ` <1541066031-29125-1-git-send-email-gavin.hu@arm.com>
     [not found]   ` <1540981587-88590-1-git-send-email-gavin.hu@arm.com>
     [not found]     ` <1540956945-211373-1-git-send-email-gavin.hu@arm.com>
2018-10-31 10:26       ` [dpdk-stable] [PATCH v3 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31 22:07         ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2018-11-01  9:56           ` Gavin Hu (Arm Technology China)
2018-10-31 10:26       ` [dpdk-stable] [PATCH v3 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-01  9:53     ` [dpdk-stable] [PATCH v4 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-01  9:53     ` [dpdk-stable] [PATCH v4 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-01 17:26       ` Stephen Hemminger
2018-11-02  0:53         ` Gavin Hu (Arm Technology China)
2018-11-02  4:30           ` Honnappa Nagarahalli
2018-11-02  7:15             ` Gavin Hu (Arm Technology China)
2018-11-02  9:36               ` Thomas Monjalon
2018-11-02 11:23                 ` Gavin Hu (Arm Technology China)
2018-11-02 11:21   ` [dpdk-stable] [PATCH v5 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-05  9:30     ` Olivier Matz
2018-11-02 11:21   ` [dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-02 11:43     ` Bruce Richardson
2018-11-03  1:19       ` Gavin Hu (Arm Technology China)
2018-11-03  9:34         ` Honnappa Nagarahalli
2018-11-05 13:17           ` Thomas Monjalon
2018-11-05 13:41             ` Jerin Jacob
2018-11-05  9:44         ` Olivier Matz
2018-11-05 13:36           ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180929104857.GA30457@jerin \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Ola.Liljedahl@arm.com \
    --cc=dev@dpdk.org \
    --cc=gavin.hu@arm.com \
    --cc=nd@arm.com \
    --cc=stable@dpdk.org \
    --cc=steve.capper@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).