From patchwork Thu Sep 3 15:16:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bill Fischofer X-Patchwork-Id: 53034 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f69.google.com (mail-la0-f69.google.com [209.85.215.69]) by patches.linaro.org (Postfix) with ESMTPS id 25DA622E23 for ; Thu, 3 Sep 2015 15:21:01 +0000 (UTC) Received: by laeb10 with SMTP id b10sf18065566lae.1 for ; Thu, 03 Sep 2015 08:21:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:delivered-to:from:to:date :message-id:in-reply-to:references:subject:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:list-subscribe :mime-version:content-type:content-transfer-encoding:errors-to :sender:x-original-sender:x-original-authentication-results :mailing-list; bh=IEMCgsZyhj2eXZdEqGpPq5dCAQarGvSDYQKY3k3mRX4=; b=HVum07R7smCLr+wxijSpanwxhbgAJGUuOb4SwopDfpnU7PWMZCx9TfJxSaH3E8DS29 UalTWttCCaw275d7hhxcK0ZUFkwxCvX9vuBafWfTYLf3WTUa9YsKGRYgN5Fkt7lT00Ix oMO+5FgIvVGPupGG2AQ/SsAtIixIgDpP1mKgqtIK49Z4mLUg++urLwpsxeib53yGZ4Xj F3OB8VArnwOpJTfr/27QHFUSxSzhWVqkvURu3C1hT927pd0jAbyCJdDxIfSYiW5oEwyR g+OZFzRSsIMudOFxI/ICrc8Dw/ts+SfgFwCd4SXHgskN3FxH1Y2M1BgQlpsoyEWOMcS5 lPbA== X-Gm-Message-State: ALoCoQmfYvuPD+PDUlySY08gxCrVBTjbPe0ehibXJh27DbNgmoxe8w3X/97M0WeVZ1RwigHXVXoN X-Received: by 10.112.149.39 with SMTP id tx7mr10894089lbb.11.1441293660102; Thu, 03 Sep 2015 08:21:00 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.45.100 with SMTP id l4ls238222lam.80.gmail; Thu, 03 Sep 2015 08:20:59 -0700 (PDT) X-Received: by 10.112.64.7 with SMTP id k7mr21208731lbs.31.1441293659882; Thu, 03 Sep 2015 08:20:59 -0700 (PDT) Received: from mail-lb0-f169.google.com (mail-lb0-f169.google.com. [209.85.217.169]) by mx.google.com with ESMTPS id zd10si23389003lbb.169.2015.09.03.08.20.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 03 Sep 2015 08:20:59 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.169 as permitted sender) client-ip=209.85.217.169; Received: by lbcjc2 with SMTP id jc2so26338666lbc.0 for ; Thu, 03 Sep 2015 08:20:59 -0700 (PDT) X-Received: by 10.112.235.130 with SMTP id um2mr22197783lbc.72.1441293659705; Thu, 03 Sep 2015 08:20:59 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.164.42 with SMTP id yn10csp1344528lbb; Thu, 3 Sep 2015 08:20:58 -0700 (PDT) X-Received: by 10.140.233.210 with SMTP id e201mr70772924qhc.88.1441293658072; Thu, 03 Sep 2015 08:20:58 -0700 (PDT) Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id w53si1156818qge.125.2015.09.03.08.20.57; Thu, 03 Sep 2015 08:20:58 -0700 (PDT) Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Received: by lists.linaro.org (Postfix, from userid 109) id 4D99A61586; Thu, 3 Sep 2015 15:20:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252 X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, URIBL_BLOCKED autolearn=disabled version=3.4.0 Received: from [127.0.0.1] (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id 7BA1761A36; Thu, 3 Sep 2015 15:17:32 +0000 (UTC) X-Original-To: lng-odp@lists.linaro.org Delivered-To: lng-odp@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id 5027A61A2A; Thu, 3 Sep 2015 15:17:26 +0000 (UTC) Received: from mail-ob0-f171.google.com (mail-ob0-f171.google.com [209.85.214.171]) by lists.linaro.org (Postfix) with ESMTPS id 2ECA461990 for ; Thu, 3 Sep 2015 15:16:51 +0000 (UTC) Received: by obqa2 with SMTP id a2so35061583obq.3 for ; Thu, 03 Sep 2015 08:16:50 -0700 (PDT) X-Received: by 10.60.50.169 with SMTP id d9mr28174223oeo.9.1441293410678; Thu, 03 Sep 2015 08:16:50 -0700 (PDT) Received: from Ubuntu15.localdomain (cpe-24-28-70-239.austin.res.rr.com. [24.28.70.239]) by smtp.gmail.com with ESMTPSA id e73sm1525294oib.3.2015.09.03.08.16.50 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 03 Sep 2015 08:16:50 -0700 (PDT) From: Bill Fischofer To: lng-odp@lists.linaro.org, maxim.uvarov@linaro.org Date: Thu, 3 Sep 2015 10:16:38 -0500 Message-Id: <1441293400-28777-7-git-send-email-bill.fischofer@linaro.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1441293400-28777-1-git-send-email-bill.fischofer@linaro.org> References: <1441293400-28777-1-git-send-email-bill.fischofer@linaro.org> X-Topics: patch Subject: [lng-odp] [PATCHv2 6/8] linux-generic: queue: correct handling of reorder completion X-BeenThere: lng-odp@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , MIME-Version: 1.0 Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: bill.fischofer@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.169 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 Fix a race condition that could cause events to be stuck in an ordered queue's reorder queue. Signed-off-by: Bill Fischofer --- .../linux-generic/include/odp_queue_internal.h | 36 ++++++++++++---- platform/linux-generic/odp_queue.c | 48 +++++++++------------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/platform/linux-generic/include/odp_queue_internal.h b/platform/linux-generic/include/odp_queue_internal.h index f285ea3..48576bc 100644 --- a/platform/linux-generic/include/odp_queue_internal.h +++ b/platform/linux-generic/include/odp_queue_internal.h @@ -284,18 +284,38 @@ static inline int reorder_deq(queue_entry_t *queue, return deq_count; } -static inline int reorder_complete(odp_buffer_hdr_t *reorder_buf) +static inline void reorder_complete(queue_entry_t *origin_qe, + odp_buffer_hdr_t **reorder_buf_return, + odp_buffer_hdr_t **placeholder_buf, + int placeholder_append) { - odp_buffer_hdr_t *next_buf = reorder_buf->next; - uint64_t order = reorder_buf->order; + odp_buffer_hdr_t *reorder_buf = origin_qe->s.reorder_head; + odp_buffer_hdr_t *next_buf; + + *reorder_buf_return = NULL; + if (!placeholder_append) + *placeholder_buf = NULL; - while (reorder_buf->flags.sustain && - next_buf && next_buf->order == order) { - reorder_buf = next_buf; + while (reorder_buf && + reorder_buf->order <= origin_qe->s.order_out) { next_buf = reorder_buf->next; - } - return !reorder_buf->flags.sustain; + if (!reorder_buf->target_qe) { + origin_qe->s.reorder_head = next_buf; + reorder_buf->next = *placeholder_buf; + *placeholder_buf = reorder_buf; + + reorder_buf = next_buf; + order_release(origin_qe, 1); + } else if (reorder_buf->flags.sustain) { + reorder_buf = next_buf; + } else { + *reorder_buf_return = origin_qe->s.reorder_head; + origin_qe->s.reorder_head = + origin_qe->s.reorder_head->next; + break; + } + } } static inline void get_queue_order(queue_entry_t **origin_qe, uint64_t *order, diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c index 87483b9..15abd93 100644 --- a/platform/linux-generic/odp_queue.c +++ b/platform/linux-generic/odp_queue.c @@ -458,18 +458,10 @@ int queue_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int sustain) order_release(origin_qe, release_count + placeholder_count); /* Now handle any unblocked complete buffers destined for - * other queues. Note that these must be complete because - * otherwise another thread is working on it and is - * responsible for resolving order when it is complete. + * other queues, appending placeholder bufs as needed. */ UNLOCK(&queue->s.lock); - - if (reorder_buf && - reorder_buf->order <= origin_qe->s.order_out && - reorder_complete(reorder_buf)) - origin_qe->s.reorder_head = reorder_buf->next; - else - reorder_buf = NULL; + reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, 1); UNLOCK(&origin_qe->s.lock); if (reorder_buf) @@ -827,12 +819,7 @@ int queue_pktout_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, order_release(origin_qe, release_count + placeholder_count); /* Now handle sends to other queues that are ready to go */ - if (reorder_buf && - reorder_buf->order <= origin_qe->s.order_out && - reorder_complete(reorder_buf)) - origin_qe->s.reorder_head = reorder_buf->next; - else - reorder_buf = NULL; + reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, 1); /* We're fully done with the origin_qe at last */ UNLOCK(&origin_qe->s.lock); @@ -910,23 +897,28 @@ int release_order(queue_entry_t *origin_qe, uint64_t order, order_release(origin_qe, 1); /* Check if this release allows us to unblock waiters. - * Note that we can only process complete waiters since - * if the sustain bit is set for a buffer this means that - * some other thread is working on it and will be - * responsible for resolving order when it is complete. + * At the point of this call, the reorder list may contain + * zero or more placeholders that need to be freed, followed + * by zero or one complete reorder buffer chain. */ - reorder_buf = origin_qe->s.reorder_head; - - if (reorder_buf && - reorder_buf->order <= origin_qe->s.order_out && - reorder_complete(reorder_buf)) - origin_qe->s.reorder_head = reorder_buf->next; - else - reorder_buf = NULL; + reorder_complete(origin_qe, &reorder_buf, + &placeholder_buf_hdr, 0); + /* Now safe to unlock */ UNLOCK(&origin_qe->s.lock); + + /* If reorder_buf has a target, do the enq now */ if (reorder_buf) queue_enq_internal(reorder_buf); + + while (placeholder_buf_hdr) { + odp_buffer_hdr_t *placeholder_next = + placeholder_buf_hdr->next; + + odp_buffer_free(placeholder_buf_hdr->handle.handle); + placeholder_buf_hdr = placeholder_next; + } + return 0; }