Message ID | 20230821085806.3062613-1-david.marchand@redhat.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 83180430BE; Mon, 21 Aug 2023 10:58:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 73E914282D; Mon, 21 Aug 2023 10:58:15 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id C4B2F40A7D for <dev@dpdk.org>; Mon, 21 Aug 2023 10:58:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692608293; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZeGqV9UH154VzeOFIRVpPjkwNI6CvZ7tyllTqOb8MU4=; b=Dk4MN+e+r2W4ltsdTgvpU+ZmsmT9MB5xLc78+VKqKn5wH+Y2qD96A69BcZmB/Ah73LJFWR PR1vodAIYS/6AKVS6XDRSRJAdO6Ux777KVqWwpBsEiAg0XW5V1wBHM365y9pgodFoajLul TGoz/K8umHfCsHciOnv7IhzmiGi4nbE= Received: from mimecast-mx02.redhat.com (66.187.233.73 [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-98-QyY0-XXuOF6AnIFFafRRhg-1; Mon, 21 Aug 2023 04:58:11 -0400 X-MC-Unique: QyY0-XXuOF6AnIFFafRRhg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 03D453815EE5; Mon, 21 Aug 2023 08:58:11 +0000 (UTC) Received: from dmarchan.redhat.com (unknown [10.45.226.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 69388492C13; Mon, 21 Aug 2023 08:58:10 +0000 (UTC) From: David Marchand <david.marchand@redhat.com> To: dev@dpdk.org Cc: probb@iol.unh.edu Subject: [PATCH v3 0/3] Release ethdev shared memory on port cleanup Date: Mon, 21 Aug 2023 10:58:03 +0200 Message-ID: <20230821085806.3062613-1-david.marchand@redhat.com> In-Reply-To: <20230818091321.2404089-1-david.marchand@redhat.com> References: <20230818091321.2404089-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
Series |
Release ethdev shared memory on port cleanup
|
|
Message
David Marchand
Aug. 21, 2023, 8:58 a.m. UTC
This series was triggered after investigating why the eal_flags_file_prefix_autotest unit test was failing in the case of statically built binaries [1]). For now, I went with a simple (naive) approach and put all accesses to the shared data under a single lock: ethdev maintainers, it is your turn to shine and give me reasons why we should keep the locks the way they were ;-). And let's see what the CI reports... 1: https://inbox.dpdk.org/dev/20230816153439.551501-12-bruce.richardson@intel.com/T/#m0e4c23f7be80bbdac076a387f4a2f9094dd07e0a
Comments
21/08/2023 10:58, David Marchand: > This series was triggered after investigating why the > eal_flags_file_prefix_autotest unit test was failing in the case of > statically built binaries [1]). > > For now, I went with a simple (naive) approach and put all accesses to the > shared data under a single lock: ethdev maintainers, it is your turn to > shine and give me reasons why we should keep the locks the way they > were ;-). I think the reasons are: - we wanted to call rte_spinlock_init() - we didn't want to allocate an ethdev lock in EAL memory config How eliminating a lock is making the last patch easier exactly?
On Thu, Aug 31, 2023 at 6:14 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > 21/08/2023 10:58, David Marchand: > > This series was triggered after investigating why the > > eal_flags_file_prefix_autotest unit test was failing in the case of > > statically built binaries [1]). > > > > For now, I went with a simple (naive) approach and put all accesses to the > > shared data under a single lock: ethdev maintainers, it is your turn to > > shine and give me reasons why we should keep the locks the way they > > were ;-). > > I think the reasons are: > - we wanted to call rte_spinlock_init() > - we didn't want to allocate an ethdev lock in EAL memory config Hum, it is a choice of implementation, not a list of locking requirements. As to why we would not want ethdev lock in EAL, I can understand the concern. This could be enhanced with a new service provided by EAL to register some space in the shared configuration for use by libraries. However, seeing how the mempool and timer libraries already had one lock stored there, I assumed it was ok to do the same. > How eliminating a lock is making the last patch easier exactly? Let's say that my goal is to cleanup resources once a DPDK application exits (no hugepage files left). Here, it is a first step in that direction, with releasing ethdev shared mem data. It is not a complete solution, as other device classes probably have the same issues of leaving some shared data behind. The ethdev shared memory hosts ethdev ports, and the ownership notion. The current code implicitly relies on the assumption that the shared memory will never go away. So I had to revisit and protect places accessing this shared memory, and having one shared lock for protecting access was necessary. But doing so, the ownership lock would be nested in this global lock while doing nothing more that the ethdev shared data lock. I will enhance the patches description.