From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 96034A04AB; Fri, 8 Nov 2019 07:02:40 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 031671BF57; Fri, 8 Nov 2019 07:02:40 +0100 (CET) Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by dpdk.org (Postfix) with ESMTP id BBBF51BF51 for ; Fri, 8 Nov 2019 07:02:38 +0100 (CET) Received: by mail-io1-f53.google.com with SMTP id x21so5114755iol.2 for ; Thu, 07 Nov 2019 22:02:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Nm/yYZpI+m5tHGbTc3XvrF5DCEF4nOZBC123Zy5n0qU=; b=YtHvoXQAZ2dUAMfK3gHoDymHus5tyT5lHmsux1CRYJd0prJj8AmDPLzxh/OcADfJGR jUtXvgXkexJiqSCzFJFIPpFiD6xrKQKlPAanzHJtXq9s1gXIgT+3iUvLBDLZET+qxgI4 p+oAfFiud7rI4gSO/1zRVwORgC5w3YPB/N2puGVDIq88TsCjnDmZw7mLTVW986mS6XQw XN1HIVQbRiqZm2nNxJk7VyrMx2LT/iYHQNs2W+nCUw2QiQD9bW4sxW6pjLc2AVdUIsrv QVmROLy35HYudqQz4f9QoH0GNFvqgPXm/OH4Da3blG6YujoJbegVKSbF1EtpoqVpZAjH JDbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Nm/yYZpI+m5tHGbTc3XvrF5DCEF4nOZBC123Zy5n0qU=; b=qhFHXo7I4ON9sqan7f/F9Wum6Pq3caf4Q1tQ+ScfyVyBhy2d/yeFZppa2eZYMyhEjE sEUAlaAQr7r0r98zUWv09finGJXn6H3zVp/xQb3R+R6ZQBDGtS0nuw05E1Pj6sowqChx dAjYi4O5shnduI5MMrO5p0k7NFKm7VuZZHF/G4YCeq1NlGg65bUcGHcYikJPaUUC7Fpf 6Klfl6AuwzxpDmC16sVR88ocsv2D3LqydZCbYBXF7IZTp8yFdTn5Xv9nBU7BtPteSkPz 4e2IBy/XbBC3Zbb6TWImjbWIp6Si0Xk9YzKaWvI20979Vy/PS59wXHVxefXld7y0Eu7F YGKA== X-Gm-Message-State: APjAAAX0KIgqmRvoIRacj8zuXIzzcBq4EiTSteqqFvS1Xg9I6PGh/onV x+93FofFWHlqSKHLoQ11vFB2/6Ezx6zNzc9ucvJmrA34 X-Google-Smtp-Source: APXvYqwH5Dm4b0kZkwzZuN5F5iSsUCekucwuKuV6Ap3YYqTEW2yImGZ2+LOF7SkzezZDaMGXFjFjMSGpB2HqdtxM7JE= X-Received: by 2002:a05:6638:401:: with SMTP id q1mr8917408jap.61.1573192957898; Thu, 07 Nov 2019 22:02:37 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: kumaraparameshwaran rathinavel Date: Fri, 8 Nov 2019 11:32:26 +0530 Message-ID: To: =?UTF-8?Q?Micha=C5=82_Krawczyk?= Cc: dev@dpdk.org, "Chauskin, Igor" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] Admin Queue ENA 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Micha=C5=82, Please look at the below function, static int ena_com_wait_and_process_admin_cq_polling( struct ena_comp_ctx *comp_ctx, struct ena_com_admin_queue *admin_queue) { unsigned long flags =3D 0; u64 start_time; int ret; start_time =3D ENA_GET_SYSTEM_USECS(); while (comp_ctx->status =3D=3D ENA_CMD_SUBMITTED) { if ((ENA_GET_SYSTEM_USECS() - start_time) > ADMIN_CMD_TIMEOUT_US) { ena_trc_err("Wait for completion (polling) timeout\n"); /* ENA didn't have any completion */ ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags); admin_queue->stats.no_completion++; admin_queue->running_state =3D false; ENA_SPINLOCK_UNLOCK(admin_queue->q_lock, flags); ret =3D ENA_COM_TIMER_EXPIRED; goto err; } *ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags); ena_com_handle_admin_completion(admin_queue); ENA_SPINLOCK_UNLOCK(admin_queue->q_lock, flags);* } if (unlikely(comp_ctx->status =3D=3D ENA_CMD_ABORTED)) { ena_trc_err("Command was aborted\n"); ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags); admin_queue->stats.aborted_cmd++; ENA_SPINLOCK_UNLOCK(admin_queue->q_lock, flags); ret =3D ENA_COM_NO_DEVICE; goto err; } ENA_ASSERT(comp_ctx->status =3D=3D ENA_CMD_COMPLETED, "Invalid comp status %d\n", comp_ctx->status); ret =3D ena_com_comp_status_to_errno(comp_ctx->comp_status); err: *comp_ctxt_release(admin_queue, comp_ctx);* return ret; } This is a case where there are two threads executing admin commands. The occupied flag is set to false in the function comp_ctxt_release. Let us say there are two consumers of completion context and C1 has a completion context and the same completion context can be used by another consumer C2 even before the C1 is resetting the occupied flag. This is because the ena_com_handle_admin_completion is done under spin lock and comp_ctxt_release is not under this spin lock. Thanks, Param On Thu, Oct 24, 2019 at 2:09 PM Micha=C5=82 Krawczyk wrot= e: > sob., 19 pa=C5=BA 2019 o 20:26 kumaraparameshwaran rathinavel > napisa=C5=82(a): > > > > Hi All, > > > > In the ENA poll mode driver I see that every request in the admin queue > is > > associated with a completion context and this is preallocated during th= e > > device initialisation. When the completion context is used we check for > > occupied to be true in the 16.X version if the occupied flag is set to > true > > we assert and in the latest version I see that this is an error log. Bu= t > > there is a time window where if the completion context would be availab= le > > to the other consumer but still the old consumer did not set the occupi= ed > > to false. The new consumer holds the admin queue lock to get the > completion > > context but the update by the old consumer to set the the occupied flag > is > > not done under lock. So should we make sure that the new consumer shoul= d > > get the completion context only when the occupied flag is set to false. > Any > > thoughts on this? > > Hi Param, > > Both the producer and the consumer are holding the spinlock while > getting the completion context. If you see any situation where it > isn't (besides the release function), please let me know. > As it is protected by the lock, returning error while completion > context is occupied (and it shouldn't) it fine, as it will stop the > admin queue and allow the DPDK user application to execute the reset > of the device. > > Thanks, > Michal > > > If required I can try to make a patch where the completion context woul= d > be > > available only after setting the occupied flag to false. > > > > Thanks, > > Param. >