From: "Wang, Yipeng1" <yipeng1.wang@intel.com>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>,
Steve Capper <Steve.Capper@arm.com>,
Ola Liljedahl <Ola.Liljedahl@arm.com>, nd <nd@arm.com>,
"Gobriel, Sameh" <sameh.gobriel@intel.com>
Subject: Re: [dpdk-dev] [PATCH 3/4] hash: fix rw concurrency while moving keys
Date: Mon, 1 Oct 2018 22:56:28 +0000 [thread overview]
Message-ID: <D2C4A16CA39F7F4E8E384D204491D7A6614EA2BD@FMSMSX151.amr.corp.intel.com> (raw)
In-Reply-To: <AM6PR08MB36724E09B9AF737412D166C598EE0@AM6PR08MB3672.eurprd08.prod.outlook.com>
>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>Sent: Sunday, September 30, 2018 4:06 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
><pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; Steve Capper <Steve.Capper@arm.com>; Ola Liljedahl
><Ola.Liljedahl@arm.com>; nd <nd@arm.com>
>Subject: RE: [dpdk-dev] [PATCH 3/4] hash: fix rw concurrency while moving keys
>
>> >+ __atomic_store_n(&h->tbl_chng_cnt,
>> >+ h->tbl_chng_cnt + 1,
>> >+ __ATOMIC_RELEASE);
>> >+ /* The stores to sig_alt and sig_current should not
>> >+ * move above the store to tbl_chng_cnt.
>> >+ */
>> >+ __atomic_thread_fence(__ATOMIC_RELEASE);
>> >+
>> [Wang, Yipeng] I believe for X86 this fence should not be compiled to any
>> code, otherwise we need macros for the compile time check.
>'__atomic_thread_fence(__ATOMIC_RELEASE)' provides load-load and load-store fence [1]. Hence, it should not add any barriers for
>x86.
>
>[1] https://preshing.com/20130922/acquire-and-release-fences/
>
[Wang, Yipeng] Thanks for the link, it is very informative!
>>
>> >@@ -926,30 +957,56 @@ __rte_hash_lookup_with_hash(const struct
>> rte_hash *h, const void *key,
>> > uint32_t bucket_idx;
>> > hash_sig_t alt_hash;
>> > struct rte_hash_bucket *bkt;
>> >+ uint32_t cnt_b, cnt_a;
>> > int ret;
>> >
>> >- bucket_idx = sig & h->bucket_bitmask;
>> >- bkt = &h->buckets[bucket_idx];
>> >-
>> > __hash_rw_reader_lock(h);
>> >
>> >- /* Check if key is in primary location */
>> >- ret = search_one_bucket(h, key, sig, data, bkt);
>> >- if (ret != -1) {
>> >- __hash_rw_reader_unlock(h);
>> >- return ret;
>> >- }
>> >- /* Calculate secondary hash */
>> >- alt_hash = rte_hash_secondary_hash(sig);
>> >- bucket_idx = alt_hash & h->bucket_bitmask;
>> >- bkt = &h->buckets[bucket_idx];
>> >+ do {
>> [Wang, Yipeng] As far as I know, the MemC3 paper "MemC3: Compact and
>> Concurrent MemCache with Dumber Caching and Smarter Hashing"
>> as well as OvS cmap uses similar version counter to implement read-write
>> concurrency for hash table, but one difference is reader checks even/odd of
>> the version counter to make sure there is no concurrent writer. Could you just
>> double check and confirm that this is not needed for your implementation?
>>
>I relooked at this paper. My patch makes use of the fact that during the process of shifting the key will be present in both primary and
>secondary buckets. The check for odd version counter is not required as the full key comparison would have identified any false
>signature matches.
[Wang, Yipeng] I was thinking about another corner case and wondering if the version counter needs to be done on key deletion as well.
For example, a reader reads out the index and falsely matches a signature, before it reads out the data and the key, the key-data pair got
deleted, removed, and recycled by another writer thread. This writer partially overwrote the key (because key store is too large to be atomic).
Now, the reader begins to compare the key, and accidentally matches the key (because the key is partially written and accidentally matches), will
The reader read the wrong data out (which should have been a lookup miss)?
next prev parent reply other threads:[~2018-10-01 23:05 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-06 17:12 [dpdk-dev] [PATCH 0/4] Address reader-writer concurrency in rte_hash Honnappa Nagarahalli
2018-09-06 17:12 ` [dpdk-dev] [PATCH 1/4] hash: correct key store element alignment Honnappa Nagarahalli
2018-09-27 23:58 ` Wang, Yipeng1
2018-09-06 17:12 ` [dpdk-dev] [PATCH 2/4] hash: add memory ordering to avoid race conditions Honnappa Nagarahalli
2018-09-28 0:43 ` Wang, Yipeng1
2018-09-30 22:20 ` Honnappa Nagarahalli
2018-10-01 22:41 ` Wang, Yipeng1
2018-10-01 10:42 ` Ola Liljedahl
2018-10-02 1:52 ` Wang, Yipeng1
2018-09-06 17:12 ` [dpdk-dev] [PATCH 3/4] hash: fix rw concurrency while moving keys Honnappa Nagarahalli
2018-09-28 1:00 ` Wang, Yipeng1
2018-09-28 8:26 ` Bruce Richardson
2018-09-28 8:55 ` Van Haaren, Harry
2018-09-30 22:33 ` Honnappa Nagarahalli
2018-10-02 13:17 ` Van Haaren, Harry
2018-10-02 23:58 ` Wang, Yipeng1
2018-10-03 17:32 ` Honnappa Nagarahalli
2018-10-03 17:56 ` Wang, Yipeng1
2018-10-03 23:05 ` Ola Liljedahl
2018-10-04 3:32 ` Honnappa Nagarahalli
2018-10-04 3:54 ` Honnappa Nagarahalli
2018-10-04 19:16 ` Wang, Yipeng1
2018-09-30 23:05 ` Honnappa Nagarahalli
2018-10-01 22:56 ` Wang, Yipeng1 [this message]
2018-10-03 0:16 ` Wang, Yipeng1
2018-10-03 17:39 ` Honnappa Nagarahalli
2018-09-06 17:12 ` [dpdk-dev] [PATCH 4/4] hash: enable lock-free reader-writer concurrency Honnappa Nagarahalli
2018-09-28 1:33 ` Wang, Yipeng1
2018-10-01 4:11 ` Honnappa Nagarahalli
2018-10-01 23:54 ` Wang, Yipeng1
2018-10-11 5:24 ` Honnappa Nagarahalli
2018-09-14 21:18 ` [dpdk-dev] [PATCH 0/4] Address reader-writer concurrency in rte_hash Honnappa Nagarahalli
2018-09-26 14:36 ` Honnappa Nagarahalli
2018-09-27 23:45 ` Wang, Yipeng1
2018-09-28 21:11 ` Honnappa Nagarahalli
2018-10-02 0:30 ` Wang, Yipeng1
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=D2C4A16CA39F7F4E8E384D204491D7A6614EA2BD@FMSMSX151.amr.corp.intel.com \
--to=yipeng1.wang@intel.com \
--cc=Gavin.Hu@arm.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Ola.Liljedahl@arm.com \
--cc=Steve.Capper@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=nd@arm.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=sameh.gobriel@intel.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).