List comments

GET /api/patches/55372/comments/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 97685,
        "web_url": "http://patches.dpdk.org/comment/97685/",
        "msgid": "<BE54F058557D9A4FAC1D84E2FC6D875723527352@fmsmsx115.amr.corp.intel.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/BE54F058557D9A4FAC1D84E2FC6D875723527352@fmsmsx115.amr.corp.intel.com",
        "date": "2019-06-27T18:48:05",
        "subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
        "submitter": {
            "id": 762,
            "url": "http://patches.dpdk.org/api/people/762/",
            "name": "Carrillo, Erik G",
            "email": "erik.g.carrillo@intel.com"
        },
        "content": "> -----Original Message-----\n> From: Burakov, Anatoly\n> Sent: Tuesday, June 25, 2019 11:12 AM\n> To: dev@dpdk.org\n> Cc: Robert Sanford <rsanford@akamai.com>; Carrillo, Erik G\n> <erik.g.carrillo@intel.com>\n> Subject: [PATCH 2/2] timer: fix resource leak in finalize\n> \n> Currently, whenever timer library is initialized, the memory is leaked because\n> there is no telling when primary or secondary processes get to use the state,\n> and there is no way to initialize/deinitialize timer library state without race\n> conditions because the data itself must live in shared memory.\n> \n> However, there is now a timer library lock in the shared memory config,\n> which can be used to synchronize access to the timer library shared memory.\n> Use it to initialize/deinitialize timer library shared data in a safe way. There is\n> still a way to leak the memory (by killing one of the processes), but we can't\n> do anything about that.\n> \n> Also, update the API doc. Note that the behavior of the API itself did not\n> change - the requirement to call init in every process was simply not\n> documented explicitly.\n> \n> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\n> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\nAcked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>",
        "headers": {
            "X-Amp-File-Uploaded": "False",
            "Subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Original-To": "patchwork@dpdk.org",
            "Message-ID": "<BE54F058557D9A4FAC1D84E2FC6D875723527352@fmsmsx115.amr.corp.intel.com>",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "From": "\"Carrillo, Erik G\" <erik.g.carrillo@intel.com>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "X-MS-Has-Attach": "",
            "X-BeenThere": "dev@dpdk.org",
            "dlp-product": "dlpe-windows",
            "dlp-reaction": "no-action",
            "x-originating-ip": "[10.1.200.107]",
            "Accept-Language": "en-US",
            "CC": "Robert Sanford <rsanford@akamai.com>",
            "Date": "Thu, 27 Jun 2019 18:48:05 +0000",
            "In-Reply-To": "<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>",
            "Content-Transfer-Encoding": "quoted-printable",
            "Errors-To": "dev-bounces@dpdk.org",
            "References": "<1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com>\n\t<cover.1561478924.git.anatoly.burakov@intel.com>\n\t<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "Delivered-To": "patchwork@dpdk.org",
            "Precedence": "list",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id D71DB31FC;\n\tThu, 27 Jun 2019 20:48:09 +0200 (CEST)",
                "from mga03.intel.com (mga03.intel.com [134.134.136.65])\n\tby dpdk.org (Postfix) with ESMTP id A44442BF4\n\tfor <dev@dpdk.org>; Thu, 27 Jun 2019 20:48:08 +0200 (CEST)",
                "from orsmga001.jf.intel.com ([10.7.209.18])\n\tby orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t27 Jun 2019 11:48:07 -0700",
                "from fmsmsx107.amr.corp.intel.com ([10.18.124.205])\n\tby orsmga001.jf.intel.com with ESMTP; 27 Jun 2019 11:48:07 -0700",
                "from fmsmsx126.amr.corp.intel.com (10.18.125.43) by\n\tfmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP\n\tServer (TLS) id 14.3.439.0; Thu, 27 Jun 2019 11:48:07 -0700",
                "from fmsmsx115.amr.corp.intel.com ([169.254.4.71]) by\n\tFMSMSX126.amr.corp.intel.com ([169.254.1.119]) with mapi id\n\t14.03.0439.000; Thu, 27 Jun 2019 11:48:06 -0700"
            ],
            "MIME-Version": "1.0",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Thread-Topic": "[PATCH 2/2] timer: fix resource leak in finalize",
            "To": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>, \"dev@dpdk.org\"\n\t<dev@dpdk.org>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Content-Language": "en-US",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "Content-Type": "text/plain; charset=\"us-ascii\"",
            "X-ExtLoop1": "1",
            "dlp-version": "11.0.600.7",
            "X-Mailman-Version": "2.1.15",
            "Thread-Index": "AQHVK3Czp17BpxRqyUOlp+I2V5eb6Kav2SVw",
            "X-MS-TNEF-Correlator": "",
            "X-IronPort-AV": "E=Sophos;i=\"5.63,424,1557212400\"; d=\"scan'208\";a=\"245912217\""
        }
    },
    {
        "id": 98136,
        "web_url": "http://patches.dpdk.org/comment/98136/",
        "msgid": "<CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com",
        "date": "2019-07-04T09:10:04",
        "subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
        "submitter": {
            "id": 1173,
            "url": "http://patches.dpdk.org/api/people/1173/",
            "name": "David Marchand",
            "email": "david.marchand@redhat.com"
        },
        "content": "On Tue, Jun 25, 2019 at 6:12 PM Anatoly Burakov <anatoly.burakov@intel.com>\nwrote:\n\n> Currently, whenever timer library is initialized, the memory is leaked\n> because there is no telling when primary or secondary processes get\n> to use the state, and there is no way to initialize/deinitialize\n> timer library state without race conditions because the data itself\n> must live in shared memory.\n>\n> However, there is now a timer library lock in the shared memory\n> config, which can be used to synchronize access to the timer\n> library shared memory. Use it to initialize/deinitialize timer\n> library shared data in a safe way. There is still a way to leak\n> the memory (by killing one of the processes), but we can't do\n> anything about that.\n>\n> Also, update the API doc. Note that the behavior of the API\n> itself did not change - the requirement to call init in every\n> process was simply not documented explicitly.\n>\n> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\n> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n> ---\n>  lib/librte_timer/rte_timer.c | 41 ++++++++++++++++++++++++------------\n>  lib/librte_timer/rte_timer.h |  5 +++--\n>  2 files changed, 31 insertions(+), 15 deletions(-)\n>\n> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c\n> index dd7953922..08517c120 100644\n> --- a/lib/librte_timer/rte_timer.c\n> +++ b/lib/librte_timer/rte_timer.c\n> @@ -13,6 +13,7 @@\n>  #include <rte_atomic.h>\n>  #include <rte_common.h>\n>  #include <rte_cycles.h>\n> +#include <rte_eal_memconfig.h>\n>  #include <rte_per_lcore.h>\n>  #include <rte_memory.h>\n>  #include <rte_launch.h>\n> @@ -61,6 +62,8 @@ struct rte_timer_data {\n>  };\n>\n>  #define RTE_MAX_DATA_ELS 64\n> +static const struct rte_memzone *rte_timer_data_mz;\n> +static int *volatile rte_timer_mz_refcnt;\n>  static struct rte_timer_data *rte_timer_data_arr;\n>  static const uint32_t default_data_id;\n>  static uint32_t rte_timer_subsystem_initialized;\n> @@ -157,28 +160,30 @@ rte_timer_subsystem_init_v1905(void)\n>         int i, lcore_id;\n>         static const char *mz_name = \"rte_timer_mz\";\n>         const size_t data_arr_size =\n> -                               RTE_MAX_DATA_ELS *\n> sizeof(*rte_timer_data_arr);\n> +                       RTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr);\n> +       const size_t mem_size = data_arr_size +\n> sizeof(*rte_timer_mz_refcnt);\n>         bool do_full_init = true;\n>\n>         if (rte_timer_subsystem_initialized)\n>                 return -EALREADY;\n>\n> -reserve:\n> -       rte_errno = 0;\n> -       mz = rte_memzone_reserve_aligned(mz_name, data_arr_size,\n> SOCKET_ID_ANY,\n> -                                        0, RTE_CACHE_LINE_SIZE);\n> +       rte_mcfg_timer_lock();\n> +\n> +       mz = rte_memzone_lookup(mz_name);\n>         if (mz == NULL) {\n> -               if (rte_errno == EEXIST) {\n> -                       mz = rte_memzone_lookup(mz_name);\n> -                       if (mz == NULL)\n> -                               goto reserve;\n> -\n> -                       do_full_init = false;\n> -               } else\n> +               mz = rte_memzone_reserve_aligned(mz_name, mem_size,\n> +                               SOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);\n> +               if (mz == NULL) {\n> +                       rte_mcfg_timer_unlock();\n>                         return -ENOMEM;\n> -       }\n> +               }\n> +               do_full_init = true;\n> +       } else\n> +               do_full_init = false;\n>\n> +       rte_timer_data_mz = mz;\n>         rte_timer_data_arr = mz->addr;\n> +       rte_timer_mz_refcnt = (void *)((char *)mz->addr + data_arr_size);\n>\n>         if (do_full_init) {\n>                 for (i = 0; i < RTE_MAX_DATA_ELS; i++) {\n> @@ -195,6 +200,9 @@ rte_timer_subsystem_init_v1905(void)\n>         }\n>\n>         rte_timer_data_arr[default_data_id].internal_flags |= FL_ALLOCATED;\n> +       (*rte_timer_mz_refcnt)++;\n> +\n> +       rte_mcfg_timer_unlock();\n>\n>         rte_timer_subsystem_initialized = 1;\n>\n> @@ -210,6 +218,13 @@ rte_timer_subsystem_finalize(void)\n>         if (!rte_timer_subsystem_initialized)\n>                 return;\n>\n> +       rte_mcfg_timer_lock();\n> +\n> +       if (--(*rte_timer_mz_refcnt) == 0)\n> +               rte_memzone_free(rte_timer_data_mz);\n> +\n> +       rte_mcfg_timer_unlock();\n> +\n>         rte_timer_subsystem_initialized = 0;\n>  }\n>\n> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h\n> index 2196934b2..a7af10176 100644\n> --- a/lib/librte_timer/rte_timer.h\n> +++ b/lib/librte_timer/rte_timer.h\n> @@ -170,10 +170,11 @@ int __rte_experimental\n> rte_timer_data_dealloc(uint32_t id);\n>   * Initializes internal variables (list, locks and so on) for the RTE\n>   * timer library.\n>   *\n> + * @note\n> + *   This function must be called in every process before using the\n> library.\n> + *\n>   * @return\n>   *   - 0: Success\n> - *   - -EEXIST: Returned in secondary process when primary process has not\n> - *      yet initialized the timer subsystem\n>   *   - -ENOMEM: Unable to allocate memory needed to initialize timer\n>   *      subsystem\n>   */\n> --\n> 2.17.1\n>\n\nCan be squashed with the first patch.",
        "headers": {
            "Subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Original-To": "patchwork@dpdk.org",
            "Message-ID": "<CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com>",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "From": "David Marchand <david.marchand@redhat.com>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "X-BeenThere": "dev@dpdk.org",
            "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Eli4//rYjrGnOiszU5osoAm8nJjZyhh6/dULVoQdfP4=;\n\tb=PTsoyIcpC8P2IvDjvvfDaCmWBjplbhgP6cWIcocfm4LPQVSXEPaXJBwDJe+lruWsVi\n\tVvhs8nruHZZ+ZZAiCjpF9J0eqiWLP3DpCOA9mWh+KEBJTC1ViD84UrJhG+trGAL52Z0W\n\tRXMNt5L2jVGTL7EQ8KvvtoYS6HG7cklLqO0dY2ymP3mY9l11fO0/HuhbHW2j2kiasMt7\n\t/NrGZTUzVS62ax6Kt1nMWufBgPAJ82OMjYdyL5gg3/ZZFohhdtrtDHwSRvucOjr4iHWk\n\trWUnHSR18AEfbA+1zMIvyYc3KvBIds5LSx6X1KKtNrXrSPTqq1z+xFounq09q49uF+NT\n\tQU0Q==",
            "X-Gm-Message-State": "APjAAAU2weCyktqEx6fiFNn5dpUfi73p8mnRXO0sVxKIAnFXqM32fa1l\n\thRF9T2s3t34XzErtVO6pELPcErlQUu4xR6iGg482gQ==",
            "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15",
            "Date": "Thu, 4 Jul 2019 11:10:04 +0200",
            "In-Reply-To": "<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>",
            "X-Mailman-Version": "2.1.15",
            "Errors-To": "dev-bounces@dpdk.org",
            "References": "<1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com>\n\t<cover.1561478924.git.anatoly.burakov@intel.com>\n\t<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "Precedence": "list",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id B61831B998;\n\tThu,  4 Jul 2019 11:10:16 +0200 (CEST)",
                "from mail-vk1-f194.google.com (mail-vk1-f194.google.com\n\t[209.85.221.194]) by dpdk.org (Postfix) with ESMTP id DFC311B96E\n\tfor <dev@dpdk.org>; Thu,  4 Jul 2019 11:10:15 +0200 (CEST)",
                "by mail-vk1-f194.google.com with SMTP id m17so492553vkl.2\n\tfor <dev@dpdk.org>; Thu, 04 Jul 2019 02:10:15 -0700 (PDT)"
            ],
            "MIME-Version": "1.0",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Content-Type": "text/plain; charset=\"UTF-8\"",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "X-Received": "by 2002:a1f:144:: with SMTP id 65mr1877801vkb.53.1562231415256; \n\tThu, 04 Jul 2019 02:10:15 -0700 (PDT)",
            "Cc": "dev <dev@dpdk.org>, Robert Sanford <rsanford@akamai.com>, \n\tErik Gabriel Carrillo <erik.g.carrillo@intel.com>",
            "To": "Anatoly Burakov <anatoly.burakov@intel.com>",
            "X-Google-Smtp-Source": "APXvYqyAxJ/XU7flTRRWKt64DSBPztr9HNi3V2BYc8zyE6BmQLamGOU84WkUqFoEtweIMcHAwzEkkehyBX49/Tz6J50="
        }
    },
    {
        "id": 98152,
        "web_url": "http://patches.dpdk.org/comment/98152/",
        "msgid": "<d6645e97-d577-0169-8ecf-cb2671ffaf07@intel.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/d6645e97-d577-0169-8ecf-cb2671ffaf07@intel.com",
        "date": "2019-07-04T10:45:26",
        "subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
        "submitter": {
            "id": 4,
            "url": "http://patches.dpdk.org/api/people/4/",
            "name": "Anatoly Burakov",
            "email": "anatoly.burakov@intel.com"
        },
        "content": "On 04-Jul-19 10:10 AM, David Marchand wrote:\n> On Tue, Jun 25, 2019 at 6:12 PM Anatoly Burakov <anatoly.burakov@intel.com>\n> wrote:\n> \n>> Currently, whenever timer library is initialized, the memory is leaked\n>> because there is no telling when primary or secondary processes get\n>> to use the state, and there is no way to initialize/deinitialize\n>> timer library state without race conditions because the data itself\n>> must live in shared memory.\n>>\n>> However, there is now a timer library lock in the shared memory\n>> config, which can be used to synchronize access to the timer\n>> library shared memory. Use it to initialize/deinitialize timer\n>> library shared data in a safe way. There is still a way to leak\n>> the memory (by killing one of the processes), but we can't do\n>> anything about that.\n>>\n>> Also, update the API doc. Note that the behavior of the API\n>> itself did not change - the requirement to call init in every\n>> process was simply not documented explicitly.\n>>\n>> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\n>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n>> ---\n>>   lib/librte_timer/rte_timer.c | 41 ++++++++++++++++++++++++------------\n>>   lib/librte_timer/rte_timer.h |  5 +++--\n>>   2 files changed, 31 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c\n>> index dd7953922..08517c120 100644\n>> --- a/lib/librte_timer/rte_timer.c\n>> +++ b/lib/librte_timer/rte_timer.c\n>> @@ -13,6 +13,7 @@\n>>   #include <rte_atomic.h>\n>>   #include <rte_common.h>\n>>   #include <rte_cycles.h>\n>> +#include <rte_eal_memconfig.h>\n>>   #include <rte_per_lcore.h>\n>>   #include <rte_memory.h>\n>>   #include <rte_launch.h>\n>> @@ -61,6 +62,8 @@ struct rte_timer_data {\n>>   };\n>>\n>>   #define RTE_MAX_DATA_ELS 64\n>> +static const struct rte_memzone *rte_timer_data_mz;\n>> +static int *volatile rte_timer_mz_refcnt;\n>>   static struct rte_timer_data *rte_timer_data_arr;\n>>   static const uint32_t default_data_id;\n>>   static uint32_t rte_timer_subsystem_initialized;\n>> @@ -157,28 +160,30 @@ rte_timer_subsystem_init_v1905(void)\n>>          int i, lcore_id;\n>>          static const char *mz_name = \"rte_timer_mz\";\n>>          const size_t data_arr_size =\n>> -                               RTE_MAX_DATA_ELS *\n>> sizeof(*rte_timer_data_arr);\n>> +                       RTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr);\n>> +       const size_t mem_size = data_arr_size +\n>> sizeof(*rte_timer_mz_refcnt);\n>>          bool do_full_init = true;\n>>\n>>          if (rte_timer_subsystem_initialized)\n>>                  return -EALREADY;\n>>\n>> -reserve:\n>> -       rte_errno = 0;\n>> -       mz = rte_memzone_reserve_aligned(mz_name, data_arr_size,\n>> SOCKET_ID_ANY,\n>> -                                        0, RTE_CACHE_LINE_SIZE);\n>> +       rte_mcfg_timer_lock();\n>> +\n>> +       mz = rte_memzone_lookup(mz_name);\n>>          if (mz == NULL) {\n>> -               if (rte_errno == EEXIST) {\n>> -                       mz = rte_memzone_lookup(mz_name);\n>> -                       if (mz == NULL)\n>> -                               goto reserve;\n>> -\n>> -                       do_full_init = false;\n>> -               } else\n>> +               mz = rte_memzone_reserve_aligned(mz_name, mem_size,\n>> +                               SOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);\n>> +               if (mz == NULL) {\n>> +                       rte_mcfg_timer_unlock();\n>>                          return -ENOMEM;\n>> -       }\n>> +               }\n>> +               do_full_init = true;\n>> +       } else\n>> +               do_full_init = false;\n>>\n>> +       rte_timer_data_mz = mz;\n>>          rte_timer_data_arr = mz->addr;\n>> +       rte_timer_mz_refcnt = (void *)((char *)mz->addr + data_arr_size);\n>>\n>>          if (do_full_init) {\n>>                  for (i = 0; i < RTE_MAX_DATA_ELS; i++) {\n>> @@ -195,6 +200,9 @@ rte_timer_subsystem_init_v1905(void)\n>>          }\n>>\n>>          rte_timer_data_arr[default_data_id].internal_flags |= FL_ALLOCATED;\n>> +       (*rte_timer_mz_refcnt)++;\n>> +\n>> +       rte_mcfg_timer_unlock();\n>>\n>>          rte_timer_subsystem_initialized = 1;\n>>\n>> @@ -210,6 +218,13 @@ rte_timer_subsystem_finalize(void)\n>>          if (!rte_timer_subsystem_initialized)\n>>                  return;\n>>\n>> +       rte_mcfg_timer_lock();\n>> +\n>> +       if (--(*rte_timer_mz_refcnt) == 0)\n>> +               rte_memzone_free(rte_timer_data_mz);\n>> +\n>> +       rte_mcfg_timer_unlock();\n>> +\n>>          rte_timer_subsystem_initialized = 0;\n>>   }\n>>\n>> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h\n>> index 2196934b2..a7af10176 100644\n>> --- a/lib/librte_timer/rte_timer.h\n>> +++ b/lib/librte_timer/rte_timer.h\n>> @@ -170,10 +170,11 @@ int __rte_experimental\n>> rte_timer_data_dealloc(uint32_t id);\n>>    * Initializes internal variables (list, locks and so on) for the RTE\n>>    * timer library.\n>>    *\n>> + * @note\n>> + *   This function must be called in every process before using the\n>> library.\n>> + *\n>>    * @return\n>>    *   - 0: Success\n>> - *   - -EEXIST: Returned in secondary process when primary process has not\n>> - *      yet initialized the timer subsystem\n>>    *   - -ENOMEM: Unable to allocate memory needed to initialize timer\n>>    *      subsystem\n>>    */\n>> --\n>> 2.17.1\n>>\n> \n> Can be squashed with the first patch.\n> \n\nThis is not just search-and-replace - this is also a bugfix. I can \nsquash the search-and-replace part with the first patch, but this commit \nwill have to stay IMO.",
        "headers": {
            "X-Amp-File-Uploaded": "False",
            "Subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Original-To": "patchwork@dpdk.org",
            "Message-ID": "<d6645e97-d577-0169-8ecf-cb2671ffaf07@intel.com>",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "From": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "X-BeenThere": "dev@dpdk.org",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "Date": "Thu, 4 Jul 2019 11:45:26 +0100",
            "User-Agent": "Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.2",
            "In-Reply-To": "<CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com>",
            "MIME-Version": "1.0",
            "Errors-To": "dev-bounces@dpdk.org",
            "References": "<1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com>\n\t<cover.1561478924.git.anatoly.burakov@intel.com>\n\t<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>\n\t<CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com>",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "Delivered-To": "patchwork@dpdk.org",
            "Precedence": "list",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id CDB8C1BDFB;\n\tThu,  4 Jul 2019 12:45:30 +0200 (CEST)",
                "from mga14.intel.com (mga14.intel.com [192.55.52.115])\n\tby dpdk.org (Postfix) with ESMTP id 574D41BDE9\n\tfor <dev@dpdk.org>; Thu,  4 Jul 2019 12:45:29 +0200 (CEST)",
                "from fmsmga003.fm.intel.com ([10.253.24.29])\n\tby fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t04 Jul 2019 03:45:28 -0700",
                "from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.126])\n\t([10.237.220.126])\n\tby FMSMGA003.fm.intel.com with ESMTP; 04 Jul 2019 03:45:27 -0700"
            ],
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "To": "David Marchand <david.marchand@redhat.com>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Content-Language": "en-US",
            "Content-Type": "text/plain; charset=utf-8; format=flowed",
            "X-ExtLoop1": "1",
            "X-Mailman-Version": "2.1.15",
            "Cc": "dev <dev@dpdk.org>, Robert Sanford <rsanford@akamai.com>,\n\tErik Gabriel Carrillo <erik.g.carrillo@intel.com>",
            "Content-Transfer-Encoding": "7bit",
            "X-IronPort-AV": "E=Sophos;i=\"5.63,450,1557212400\"; d=\"scan'208\";a=\"172398509\""
        }
    },
    {
        "id": 98156,
        "web_url": "http://patches.dpdk.org/comment/98156/",
        "msgid": "<CAJFAV8zzpVYQHCWX7uiR8xMbe8aPAtdVin66F8aNLvdBkfzpSw@mail.gmail.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/CAJFAV8zzpVYQHCWX7uiR8xMbe8aPAtdVin66F8aNLvdBkfzpSw@mail.gmail.com",
        "date": "2019-07-04T10:50:25",
        "subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
        "submitter": {
            "id": 1173,
            "url": "http://patches.dpdk.org/api/people/1173/",
            "name": "David Marchand",
            "email": "david.marchand@redhat.com"
        },
        "content": "On Thu, Jul 4, 2019 at 12:45 PM Burakov, Anatoly <anatoly.burakov@intel.com>\nwrote:\n\n> On 04-Jul-19 10:10 AM, David Marchand wrote:\n> > On Tue, Jun 25, 2019 at 6:12 PM Anatoly Burakov <\n> anatoly.burakov@intel.com>\n> > wrote:\n> >\n> >> Currently, whenever timer library is initialized, the memory is leaked\n> >> because there is no telling when primary or secondary processes get\n> >> to use the state, and there is no way to initialize/deinitialize\n> >> timer library state without race conditions because the data itself\n> >> must live in shared memory.\n> >>\n> >> However, there is now a timer library lock in the shared memory\n> >> config, which can be used to synchronize access to the timer\n> >> library shared memory. Use it to initialize/deinitialize timer\n> >> library shared data in a safe way. There is still a way to leak\n> >> the memory (by killing one of the processes), but we can't do\n> >> anything about that.\n> >>\n> >> Also, update the API doc. Note that the behavior of the API\n> >> itself did not change - the requirement to call init in every\n> >> process was simply not documented explicitly.\n> >>\n> >> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\n> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n> >> ---\n> >>   lib/librte_timer/rte_timer.c | 41 ++++++++++++++++++++++++------------\n> >>   lib/librte_timer/rte_timer.h |  5 +++--\n> >>   2 files changed, 31 insertions(+), 15 deletions(-)\n> >>\n> >> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c\n> >> index dd7953922..08517c120 100644\n> >> --- a/lib/librte_timer/rte_timer.c\n> >> +++ b/lib/librte_timer/rte_timer.c\n> >> @@ -13,6 +13,7 @@\n> >>   #include <rte_atomic.h>\n> >>   #include <rte_common.h>\n> >>   #include <rte_cycles.h>\n> >> +#include <rte_eal_memconfig.h>\n> >>   #include <rte_per_lcore.h>\n> >>   #include <rte_memory.h>\n> >>   #include <rte_launch.h>\n> >> @@ -61,6 +62,8 @@ struct rte_timer_data {\n> >>   };\n> >>\n> >>   #define RTE_MAX_DATA_ELS 64\n> >> +static const struct rte_memzone *rte_timer_data_mz;\n> >> +static int *volatile rte_timer_mz_refcnt;\n> >>   static struct rte_timer_data *rte_timer_data_arr;\n> >>   static const uint32_t default_data_id;\n> >>   static uint32_t rte_timer_subsystem_initialized;\n> >> @@ -157,28 +160,30 @@ rte_timer_subsystem_init_v1905(void)\n> >>          int i, lcore_id;\n> >>          static const char *mz_name = \"rte_timer_mz\";\n> >>          const size_t data_arr_size =\n> >> -                               RTE_MAX_DATA_ELS *\n> >> sizeof(*rte_timer_data_arr);\n> >> +                       RTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr);\n> >> +       const size_t mem_size = data_arr_size +\n> >> sizeof(*rte_timer_mz_refcnt);\n> >>          bool do_full_init = true;\n> >>\n> >>          if (rte_timer_subsystem_initialized)\n> >>                  return -EALREADY;\n> >>\n> >> -reserve:\n> >> -       rte_errno = 0;\n> >> -       mz = rte_memzone_reserve_aligned(mz_name, data_arr_size,\n> >> SOCKET_ID_ANY,\n> >> -                                        0, RTE_CACHE_LINE_SIZE);\n> >> +       rte_mcfg_timer_lock();\n> >> +\n> >> +       mz = rte_memzone_lookup(mz_name);\n> >>          if (mz == NULL) {\n> >> -               if (rte_errno == EEXIST) {\n> >> -                       mz = rte_memzone_lookup(mz_name);\n> >> -                       if (mz == NULL)\n> >> -                               goto reserve;\n> >> -\n> >> -                       do_full_init = false;\n> >> -               } else\n> >> +               mz = rte_memzone_reserve_aligned(mz_name, mem_size,\n> >> +                               SOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);\n> >> +               if (mz == NULL) {\n> >> +                       rte_mcfg_timer_unlock();\n> >>                          return -ENOMEM;\n> >> -       }\n> >> +               }\n> >> +               do_full_init = true;\n> >> +       } else\n> >> +               do_full_init = false;\n> >>\n> >> +       rte_timer_data_mz = mz;\n> >>          rte_timer_data_arr = mz->addr;\n> >> +       rte_timer_mz_refcnt = (void *)((char *)mz->addr +\n> data_arr_size);\n> >>\n> >>          if (do_full_init) {\n> >>                  for (i = 0; i < RTE_MAX_DATA_ELS; i++) {\n> >> @@ -195,6 +200,9 @@ rte_timer_subsystem_init_v1905(void)\n> >>          }\n> >>\n> >>          rte_timer_data_arr[default_data_id].internal_flags |=\n> FL_ALLOCATED;\n> >> +       (*rte_timer_mz_refcnt)++;\n> >> +\n> >> +       rte_mcfg_timer_unlock();\n> >>\n> >>          rte_timer_subsystem_initialized = 1;\n> >>\n> >> @@ -210,6 +218,13 @@ rte_timer_subsystem_finalize(void)\n> >>          if (!rte_timer_subsystem_initialized)\n> >>                  return;\n> >>\n> >> +       rte_mcfg_timer_lock();\n> >> +\n> >> +       if (--(*rte_timer_mz_refcnt) == 0)\n> >> +               rte_memzone_free(rte_timer_data_mz);\n> >> +\n> >> +       rte_mcfg_timer_unlock();\n> >> +\n> >>          rte_timer_subsystem_initialized = 0;\n> >>   }\n> >>\n> >> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h\n> >> index 2196934b2..a7af10176 100644\n> >> --- a/lib/librte_timer/rte_timer.h\n> >> +++ b/lib/librte_timer/rte_timer.h\n> >> @@ -170,10 +170,11 @@ int __rte_experimental\n> >> rte_timer_data_dealloc(uint32_t id);\n> >>    * Initializes internal variables (list, locks and so on) for the RTE\n> >>    * timer library.\n> >>    *\n> >> + * @note\n> >> + *   This function must be called in every process before using the\n> >> library.\n> >> + *\n> >>    * @return\n> >>    *   - 0: Success\n> >> - *   - -EEXIST: Returned in secondary process when primary process has\n> not\n> >> - *      yet initialized the timer subsystem\n> >>    *   - -ENOMEM: Unable to allocate memory needed to initialize timer\n> >>    *      subsystem\n> >>    */\n> >> --\n> >> 2.17.1\n> >>\n> >\n> > Can be squashed with the first patch.\n> >\n>\n> This is not just search-and-replace - this is also a bugfix. I can\n> squash the search-and-replace part with the first patch, but this commit\n> will have to stay IMO.\n>\n\nYes this is not search and replace, but the first patch is there for the\nbugfix.\nThere are no other uses of the newly introduced API, so the whole is a\nsingle change to me.\n\nApart from that, I am ok with the changes, you can add my review tag.",
        "headers": {
            "Subject": "Re: [dpdk-dev] [PATCH 2/2] timer: fix resource leak in finalize",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Original-To": "patchwork@dpdk.org",
            "Message-ID": "<CAJFAV8zzpVYQHCWX7uiR8xMbe8aPAtdVin66F8aNLvdBkfzpSw@mail.gmail.com>",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "From": "David Marchand <david.marchand@redhat.com>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "X-BeenThere": "dev@dpdk.org",
            "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=wzAq8uyCGejrgeZW0G/upi5yJwAPilKSl7nXQgT8zsc=;\n\tb=qJPeeWqsEweEBu+t3/podZ9IPgyk6ceP+fLinyO1p0IhNGSJ7ii9kzIkQZuDmUpWsg\n\tf8W027XOsAXXhCdMk3hf38ajk/fh5EVhM3BGUFozYpupVLKODDECIC1nA0cR8dpt32Oy\n\tJ+wTDWllPDVMSuXv9ynT9lL8eWkELRN3Wu+vnz248y+AA4J0Pt0Hu3Q7FmpRbw8g62kK\n\tCpQrwvcgM0453y3arpE+K3jkFlVqDr7rlUHQa/f79eai8SBS998lgrPf7hCvekRV8l4J\n\tyKmPGdqIprOg6FO2K4x99sF3B9K3nCrLw/MO33zpqQDNwS1IyMYMGHG0QuKGdv4Y3RT/\n\t+q+A==",
            "X-Gm-Message-State": "APjAAAWRkIfvRQ7rbTgS5fKE7e2N915tpa1T/GeVagMhL+lUNnvJE4sC\n\tFZVzwiXr9S5kTws/Bs/HfJlLwMItta1nMag3djcq5g==",
            "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15",
            "Date": "Thu, 4 Jul 2019 12:50:25 +0200",
            "In-Reply-To": "<d6645e97-d577-0169-8ecf-cb2671ffaf07@intel.com>",
            "X-Mailman-Version": "2.1.15",
            "Errors-To": "dev-bounces@dpdk.org",
            "References": "<1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com>\n\t<cover.1561478924.git.anatoly.burakov@intel.com>\n\t<5c7403e06efccd2c8210ce811fa16c8e56e084b0.1561478924.git.anatoly.burakov@intel.com>\n\t<CAJFAV8yooQOON8qas9WuCPTExO+0jMk-KVQn8AstoJhKsMQGaw@mail.gmail.com>\n\t<d6645e97-d577-0169-8ecf-cb2671ffaf07@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "Precedence": "list",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 7EB951BDED;\n\tThu,  4 Jul 2019 12:50:38 +0200 (CEST)",
                "from mail-ua1-f66.google.com (mail-ua1-f66.google.com\n\t[209.85.222.66]) by dpdk.org (Postfix) with ESMTP id ADCE41BDE9\n\tfor <dev@dpdk.org>; Thu,  4 Jul 2019 12:50:36 +0200 (CEST)",
                "by mail-ua1-f66.google.com with SMTP id a97so990373uaa.9\n\tfor <dev@dpdk.org>; Thu, 04 Jul 2019 03:50:36 -0700 (PDT)"
            ],
            "MIME-Version": "1.0",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Content-Type": "text/plain; charset=\"UTF-8\"",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "X-Received": "by 2002:ab0:b99:: with SMTP id c25mr21223521uak.53.1562237436047;\n\tThu, 04 Jul 2019 03:50:36 -0700 (PDT)",
            "Cc": "dev <dev@dpdk.org>, Robert Sanford <rsanford@akamai.com>, \n\tErik Gabriel Carrillo <erik.g.carrillo@intel.com>,\n\tThomas Monjalon <thomas@monjalon.net>",
            "To": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>",
            "X-Google-Smtp-Source": "APXvYqwLk8O7bMSMSllH/6fY2d8fjayNmVSiylRWaN1EyqEI33sKj7/gzPK+tMU2w98n1Id8KPCpYAkT4lqGR+yS7gM="
        }
    }
]