From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f194.google.com (mail-pf0-f194.google.com [209.85.192.194]) by dpdk.org (Postfix) with ESMTP id 6202A1B5A5; Thu, 12 Jul 2018 04:44:25 +0200 (CEST) Received: by mail-pf0-f194.google.com with SMTP id s21-v6so19657699pfm.6; Wed, 11 Jul 2018 19:44:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=U0bvljTxoKLZ68kor4GrqPZcdbpWvW4KcKZXZOQ4/M0=; b=tWIbxNp81YCmkO7V29RECAL3JxY6U5Gs0ED4fzI7l4UY49FqPGFmjq6Q0uFomIbACy 99FTUbsLKR2p+t+3O7kPyIQj7jEVQSk7JlOsxfWb7ZeN+LXgUfX8NXqiQ7j5ONoj+DT+ sVg2sU1eCE+/y3XbmK5HjK2hIgajIFQ+QizyDSJJow0IQleR/wm5ynlxl91SzL5PWBE1 NO6VAhrDQo4hPaKBb+M2YYT28ZWCTCFyBjEnRmLsPXr5VTAuGhx/ADXmSwArHicLpRJY leBFnQwlnc/aTW3Zuls8G0MscciRUUgg4LWkcF9KhGCm5Qg4lxsP4x/r3O54NHsmBPgl VngQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=U0bvljTxoKLZ68kor4GrqPZcdbpWvW4KcKZXZOQ4/M0=; b=MjyKQZco3DhCdKy52l5DY58Dz8jthmV+Dny2gZ8dIkGTFz+YkL+75qMGIdn42OrlAJ EMeFivdytYOYpoeOciKh7Fizij13mC1DBZM1UatcniCLfTVBQK2VGsAjrB4RS/DDzDjL YtZqoBx5NVX5jEzUbJpq2Nf7aLVRmNQnMur4F3vFW0tVkFiHTooKDkfYiGOjiU/9AqDR abFHETA3QaGpq708Wt1y127lDEcrzDqK2l4R9wndjn+e45Ng/Vvoicfx9Tp4ofkptDun BqJCEWr/Dq4sc1Et0RumjYAdCfZiJIP71AeNeN3pBPI+UFHZrJEApo8yw0U53KNP4ut4 KS6w== X-Gm-Message-State: AOUpUlFdW4uOhWWoidx+CSOgUYtq6/xUABy8YHevNEljzhElGIufNXSD GMlRE3SngE5w2P2w9kOAcwGT7g== X-Google-Smtp-Source: AAOMgpc1ZRMKNmEcbVAISOilePX/cyIQAox/QF/pnDkLGbFmEno/fivPHfskwWFgKBKz/os7eY+zkA== X-Received: by 2002:a63:1c13:: with SMTP id c19-v6mr398841pgc.332.1531363463751; Wed, 11 Jul 2018 19:44:23 -0700 (PDT) Received: from takeshi-no-air.dhcp.hakozaki.ibm.com (sg-fw-ice-redblue-p6.sagamino.jp.ibm.com. [203.141.91.15]) by smtp.gmail.com with ESMTPSA id u17-v6sm71906480pfa.176.2018.07.11.19.44.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 11 Jul 2018 19:44:23 -0700 (PDT) From: Takeshi Yoshimura To: dev@dpdk.org Cc: Takeshi Yoshimura , stable@dpdk.org, Takeshi Yoshimura Date: Thu, 12 Jul 2018 11:44:14 +0900 Message-Id: <20180712024414.4756-1-t.yoshimura8869@gmail.com> X-Mailer: git-send-email 2.15.1 Subject: [dpdk-dev] [PATCH] rte_ring: fix racy dequeue/enqueue in ppc64 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: Thu, 12 Jul 2018 02:44:25 -0000 SPDK blobfs encountered a crash around rte_ring dequeues in ppc64. It uses a single consumer and multiple producers for a rte_ring. The problem was a load-load reorder in rte_ring_sc_dequeue_bulk(). The reordered loads happened on r->prod.tail in __rte_ring_move_cons_head() (rte_ring_generic.h) and ring[idx] in DEQUEUE_PTRS() (rte_ring.h). They have a load-load control dependency, but the code does not satisfy it. Note that they are not reordered if __rte_ring_move_cons_head() with is_sc != 1 because cmpset invokes a read barrier. The paired stores on these loads are in ENQUEUE_PTRS() and update_tail(). Simplified code around the reorder is the following. Consumer Producer load idx[ring] store idx[ring] store r->prod.tail load r->prod.tail In this case, the consumer loads old idx[ring] and confirms the load is valid with the new r->prod.tail. I added a read barrier in the case where __IS_SC is passed to __rte_ring_move_cons_head(). I also fixed __rte_ring_move_prod_head() to avoid similar problems with a single producer. Cc: stable@dpdk.org Signed-off-by: Takeshi Yoshimura --- lib/librte_ring/rte_ring_generic.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h index ea7dbe5b9..477326180 100644 --- a/lib/librte_ring/rte_ring_generic.h +++ b/lib/librte_ring/rte_ring_generic.h @@ -90,9 +90,10 @@ __rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp, return 0; *new_head = *old_head + n; - if (is_sp) + if (is_sp) { + rte_smp_rmb(); r->prod.head = *new_head, success = 1; - else + } else success = rte_atomic32_cmpset(&r->prod.head, *old_head, *new_head); } while (unlikely(success == 0)); @@ -158,9 +159,10 @@ __rte_ring_move_cons_head(struct rte_ring *r, unsigned int is_sc, return 0; *new_head = *old_head + n; - if (is_sc) + if (is_sc) { + rte_smp_rmb(); r->cons.head = *new_head, success = 1; - else + } else success = rte_atomic32_cmpset(&r->cons.head, *old_head, *new_head); } while (unlikely(success == 0)); -- 2.17.1