From: Gavin Hu <Gavin.Hu@arm.com>
To: Andy Green <andy@warmcat.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/2] ring: fix declaration after code
Date: Mon, 28 May 2018 08:54:15 +0000 [thread overview]
Message-ID: <VI1PR08MB31672FB58D656E8066025E5C8F6E0@VI1PR08MB3167.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <206e8a27-9039-17ac-315f-fd8f52230b4c@warmcat.com>
-----Original Message-----
From: Andy Green <andy@warmcat.com>
Sent: Monday, May 28, 2018 4:47 PM
To: Gavin Hu <Gavin.Hu@arm.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] ring: fix declaration after code
On 05/28/2018 04:15 PM, Gavin Hu wrote:
> do {
> +const uint32_t cons_tail = r->cons.tail;
> +
> /* Reset n to the initial burst count */
> n = max;
>
> *old_head = __atomic_load_n(&r->prod.head,
> __ATOMIC_ACQUIRE);
> -const uint32_t cons_tail = r->cons.tail;
> +
> [Gavin Hu] The ACQUIRE and RELEASE pair protects anything that between the two must be visible to other threads when they perform an acquire operation on the same memory address. Your changes broke this semantics. I advise to move the declaration before and keep the assignment in the old place.
I see, thanks for the tip.
How about just get rid of this temp altogether if access to it is locked during this sequence anyway? It's not like we had to sample it after the lock then, or it's bringing anything else to the party.
So instead of cons_tail / prod_tail at all, replace directly with
r->cons.tail / r->prod.tail at the single usage for each.
[Gavin Hu] I think it is ok.
-Andy
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
next prev parent reply other threads:[~2018-05-28 8:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-28 2:29 [dpdk-dev] [PATCH 0/2] dpdk header fixes for aarch64 Andy Green
2018-05-28 2:29 ` [dpdk-dev] [PATCH 1/2] ring: fix declaration after code Andy Green
2018-05-28 8:15 ` Gavin Hu
2018-05-28 8:46 ` Andy Green
2018-05-28 8:54 ` Gavin Hu [this message]
2018-05-28 2:29 ` [dpdk-dev] [PATCH 2/2] ring: fix sign conversion warning Andy Green
2018-05-28 9:03 ` [dpdk-dev] [PATCH v2 0/2] dpdk header fixes for aarch64 Andy Green
2018-05-28 9:03 ` [dpdk-dev] [PATCH v2 1/2] ring: fix declaration after code Andy Green
2018-07-16 7:15 ` Olivier Matz
2018-05-28 9:03 ` [dpdk-dev] [PATCH v2 2/2] ring: fix sign conversion warning Andy Green
2018-07-16 7:16 ` Olivier Matz
2018-07-26 13:30 ` [dpdk-dev] [PATCH v2 0/2] dpdk header fixes for aarch64 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=VI1PR08MB31672FB58D656E8066025E5C8F6E0@VI1PR08MB3167.eurprd08.prod.outlook.com \
--to=gavin.hu@arm.com \
--cc=andy@warmcat.com \
--cc=dev@dpdk.org \
/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).