List comments

GET /api/patches/40749/comments/?order=-id
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 82419,
        "web_url": "http://patches.dpdk.org/comment/82419/",
        "msgid": "<039ED4275CED7440929022BC67E706115323AA7F@SHSMSX103.ccr.corp.intel.com>",
        "date": "2018-06-20T04:00:01",
        "subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
        "submitter": {
            "id": 504,
            "url": "http://patches.dpdk.org/api/people/504/",
            "name": "Zhang, Qi Z",
            "email": "qi.z.zhang@intel.com"
        },
        "content": "Hi Anatoly:\n\tSorry to miss this email and reply late.\n\n> -----Original Message-----\n> From: Burakov, Anatoly\n> Sent: Friday, June 15, 2018 11:43 PM\n> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net\n> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;\n> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh\n> <ferruh.yigit@intel.com>; Shelton, Benjamin H\n> <benjamin.h.shelton@intel.com>; Vangati, Narender\n> <narender.vangati@intel.com>\n> Subject: Re: [PATCH 05/22] ethdev: introduce device lock\n> \n> On 07-Jun-18 1:38 PM, Qi Zhang wrote:\n> > Introduce API rte_eth_dev_lock and rte_eth_dev_unlock to let\n> > application lock or unlock on specific ethdev, a locked device can't\n> > be detached, this help applicaiton to prevent unexpected device\n> > detaching, especially in multi-process envrionment.\n> >\n> > Aslo the new API let application to register a callback function which\n> > will be invoked before a device is going to be detached, the return\n> > value of the function will decide if device will continue be detached\n> > or not, this support application to do condition check at runtime.\n> >\n> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>\n> > ---\n> \n> <snip>\n> \n> > +\n> > +int\n> > +rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> > +\t\t void *user_args)\n> > +{\n> > +\tRTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);\n> > +\n> > +\tif (callback == NULL)\n> > +\t\treturn register_lock_callback(port_id, dev_is_busy, NULL);\n> > +\telse\n> > +\t\treturn register_lock_callback(port_id, callback, user_args);\n> \n> As much as i don't like seeing negative errno values as return, the rest of\n> ethdev library uses those, so this is OK :)\n> \n> > +}\n> > +\n> > +int\n> > +rte_eth_dev_unlock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> > +\t\t   void *user_args)\n> > +{\n> > +\tRTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);\n> > +\n> \n> <snip>\n> \n> > + * Also, any callback function return !0 value will prevent device be\n> > + * detached(ref. rte_eth_dev_lock and rte_eth_dev_unlock).\n> > + *\n> > + * @param port_id\n> > + *   The port identifier of the Ethernet device.\n> > + * @param user_args\n> > + *   This is parameter \"user_args\" be saved when callback function is\n> > + *   registered(rte_dev_eth_lock).\n> > + *\n> > + * @return\n> > + *   0  device is allowed be detached.\n> > + *   !0 device is not allowed be detached.\n> \n> !0 can be negative or positive. Are we expecting positive return values from\n> this API?\n\nI have no strong opinion, but if you think below or other option is better, I can change\n\n>=0  device is allowed be detached.\n<0 device is not allowed be detached.\n\n> \n> > + */\n> > +typedef int (*rte_eth_dev_lock_callback_t)(uint16_t port_id, void\n> > +*user_args);\n> > +\n> > +/**\n> > + * Lock an Ethernet Device directly or register a callback function\n> > + * for condition check at runtime, this help application to prevent\n> > + * a device be detached unexpectedly \n> > + * NOTE: Lock a device multiple times with same parmeter will\n> > +increase\n> > + * a ref_count, and corresponding unlock decrease the ref_count, the\n> > + * device will be unlocked when ref_count reach 0.\n> \n> Nitpick: \"note\" sections should be done with @note marker.\n> \n> Also, i would mention that this is a per-process lock that does not affect other\n> processes (assuming i understood the code correctly, of course...).\n\nOK, I will add more comment to explain this.\n\n> \n> > + *\n> > + * @param port_id\n> > + *   The port identifier of the Ethernet device.\n> > + * @param callback\n> > + *   !NULL the callback function will be added into a pre-detach list,\n> > + *         it will be invoked when a device is going to be detached. The\n> > + *         return value will decide if continue detach the device or not.\n> > + *   NULL  lock the device directly, basically this just regiter a empty\n> > + *         callback function(dev_is_busy) that return -EBUSY, so we can\n> > + *         handle the pre-detach check in unified way.\n> > + * @param user_args\n> > + *   parameter will be parsed to callback function, only valid when\n> > + *   callback != NULL.\n> > + * @return\n> > + *   0 on success, negative on error.\n> > + */\n> > +int rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t\n> callback,\n> > +\t\t     void *user_args);\n> \n> Nitpicks: DPDK style guide discourages using spaces as indentation (other parts\n> of this patch, and other patches have this issue as well).\n\nOK, will fix all.\n> \n> > +\n> > +/**\n> > + * Reverse operation of rte_eth_dev_lock.\n> > + *\n> > + * @param port_id\n> > + *   The port identifier of the Ethernet device.\n> > + * @param callback\n> > + *   NULL  decrease the ref_count of default callback function.\n> > + *   !NULL decrease the ref_count of specific callback with matched\n> > + *         user_args.\n> > + * @param user_args\n> > + *   parameter to match, only valid when callback != NULL.\n> > + * @return\n> > + *   0 on success, negative on error.\n> > + */\n> > +int rte_eth_dev_unlock(uint16_t port_id, rte_eth_dev_lock_callback_t\n> callback,\n> > +\t\t       void *user_args);\n> > +\n> >   #ifdef __cplusplus\n> >   }\n> >   #endif\n> > diff --git a/lib/librte_ethdev/rte_ethdev_lock.c\n> > b/lib/librte_ethdev/rte_ethdev_lock.c\n> \n> rte_ethdev_lock.* seem to be internal-only files. Perhaps you should name\n> them without the rte_ prefix to indicate that they're not exported?\n> \n> > new file mode 100644\n> > index 000000000..688d1d70a\n> > --- /dev/null\n> > +++ b/lib/librte_ethdev/rte_ethdev_lock.c\n> > @@ -0,0 +1,102 @@\n> > +/* SPDX-License-Identifier: BSD-3-Clause\n> > + * Copyright(c) 2018 Intel Corporation  */ #include\n> > +\"rte_ethdev_lock.h\"\n> > +\n> > +struct lock_entry {\n> > +\tTAILQ_ENTRY(lock_entry) next;\n> > +\trte_eth_dev_lock_callback_t callback;\n> > +\tuint16_t port_id;\n> \n> <snip>\n> \n> > +register_lock_callback(uint16_t port_id,\n> > +\t\t       rte_eth_dev_lock_callback_t callback,\n> > +\t\t       void *user_args)\n> > +{\n> > +\tstruct lock_entry *le;\n> > +\n> > +\trte_spinlock_lock(&lock_entry_lock);\n> > +\n> > +\tTAILQ_FOREACH(le, &lock_entry_list, next) {\n> > +\t\tif (le->port_id == port_id &&\n> > +\t\t    le->callback == callback &&\n> > +\t\t    le->user_args == user_args)\n> > +\t\t\tbreak;\n> > +\t}\n> > +\n> > +\tif (!le) {\n> > +\t\tle = calloc(1, sizeof(struct lock_entry));\n> > +\t\tif (!le) {\n> \n> Nitpick: generally, DPDK style guide prefers \"if (value)\" or \"if (!value)\" to only\n> be reserved for boolean values, and use explicit comparison (e.g. \"if (value ==\n> NULL)\" or \"if (value == 0)\") for all other cases.\n\nOK, will fix\n> \n> --\n> Thanks,\n> Anatoly",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "dlp-version": "11.0.200.100",
            "dlp-reaction": "no-action",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-6-qi.z.zhang@intel.com>\n\t<bab2b78c-3037-661d-2e18-84e2749f8d94@intel.com>",
            "X-Mailman-Version": "2.1.15",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,245,1526367600\"; d=\"scan'208\";a=\"60663670\"",
            "From": "\"Zhang, Qi Z\" <qi.z.zhang@intel.com>",
            "x-originating-ip": "[10.239.127.40]",
            "x-titus-metadata-40": "eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjEyODAzZTgtMDljMS00ODdhLWI1MWYtNTQ3MGVlNGQ1MTY5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibGNiOFIrYnVVRnA4dENhQ3ZUWWEwYnBoVEZscVJaRG1ibjZYM3pNdWtwRmlGMk5US0xjcmNYb3RCZTF2VnoyUCJ9",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Thread-Index": "AQHUBL+GBVCVeOqaBU6hufOQOUk106RoiNIg",
            "Message-ID": "<039ED4275CED7440929022BC67E706115323AA7F@SHSMSX103.ccr.corp.intel.com>",
            "X-ExtLoop1": "1",
            "CC": "\"Ananyev, Konstantin\" <konstantin.ananyev@intel.com>, \"dev@dpdk.org\"\n\t<dev@dpdk.org>, \"Richardson, Bruce\" <bruce.richardson@intel.com>,\n\t\"Yigit, Ferruh\" <ferruh.yigit@intel.com>, \"Shelton, Benjamin H\"\n\t<benjamin.h.shelton@intel.com>, \"Vangati, Narender\"\n\t<narender.vangati@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Original-To": "patchwork@dpdk.org",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 2827D1B431;\n\tWed, 20 Jun 2018 06:00:09 +0200 (CEST)",
                "from mga07.intel.com (mga07.intel.com [134.134.136.100])\n\tby dpdk.org (Postfix) with ESMTP id 57D011B42F\n\tfor <dev@dpdk.org>; Wed, 20 Jun 2018 06:00:06 +0200 (CEST)",
                "from orsmga003.jf.intel.com ([10.7.209.27])\n\tby orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t19 Jun 2018 21:00:04 -0700",
                "from fmsmsx105.amr.corp.intel.com ([10.18.124.203])\n\tby orsmga003.jf.intel.com with ESMTP; 19 Jun 2018 21:00:04 -0700",
                "from shsmsx104.ccr.corp.intel.com (10.239.4.70) by\n\tFMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP\n\tServer (TLS) id 14.3.319.2; Tue, 19 Jun 2018 21:00:03 -0700",
                "from shsmsx103.ccr.corp.intel.com ([169.254.4.51]) by\n\tSHSMSX104.ccr.corp.intel.com ([169.254.5.87]) with mapi id\n\t14.03.0319.002; Wed, 20 Jun 2018 12:00:01 +0800"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Content-Language": "en-US",
            "Accept-Language": "en-US",
            "Content-Type": "text/plain; charset=\"utf-8\"",
            "X-MS-Has-Attach": "",
            "X-BeenThere": "dev@dpdk.org",
            "Date": "Wed, 20 Jun 2018 04:00:01 +0000",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "X-Amp-File-Uploaded": "False",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Precedence": "list",
            "Thread-Topic": "[PATCH 05/22] ethdev: introduce device lock",
            "In-Reply-To": "<bab2b78c-3037-661d-2e18-84e2749f8d94@intel.com>",
            "Errors-To": "dev-bounces@dpdk.org",
            "x-ctpclassification": "CTP_NT",
            "dlp-product": "dlpe-windows",
            "MIME-Version": "1.0",
            "Content-Transfer-Encoding": "base64",
            "To": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>, \"thomas@monjalon.net\"\n\t<thomas@monjalon.net>",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "X-MS-TNEF-Correlator": ""
        }
    },
    {
        "id": 82400,
        "web_url": "http://patches.dpdk.org/comment/82400/",
        "msgid": "<039ED4275CED7440929022BC67E706115323A32F@SHSMSX103.ccr.corp.intel.com>",
        "date": "2018-06-19T14:16:30",
        "subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
        "submitter": {
            "id": 504,
            "url": "http://patches.dpdk.org/api/people/504/",
            "name": "Zhang, Qi Z",
            "email": "qi.z.zhang@intel.com"
        },
        "content": "Hi Stephen:\n\n\n> -----Original Message-----\n> From: Stephen Hemminger [mailto:stephen@networkplumber.org]\n> Sent: Saturday, June 16, 2018 12:09 AM\n> To: Zhang, Qi Z <qi.z.zhang@intel.com>\n> Cc: thomas@monjalon.net; Burakov, Anatoly <anatoly.burakov@intel.com>;\n> Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;\n> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh\n> <ferruh.yigit@intel.com>; Shelton, Benjamin H\n> <benjamin.h.shelton@intel.com>; Vangati, Narender\n> <narender.vangati@intel.com>\n> Subject: Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock\n> \n> On Thu,  7 Jun 2018 20:38:32 +0800\n> Qi Zhang <qi.z.zhang@intel.com> wrote:\n> \n> > +/**\n> > + * Lock an Ethernet Device directly or register a callback function\n> > + * for condition check at runtime, this help application to prevent\n> > + * a device be detached unexpectly.\n> > + * NOTE: Lock a device mutliple times with same parmeter will increase\n> > + * a ref_count, and coresponding unlock decrease the ref_count, the\n> > + * device will be unlocked when ref_count reach 0.\n> > + *\n> > + * @param port_id\n> > + *   The port identifier of the Ethernet device.\n> > + * @param callback\n> > + *   !NULL the callback function will be added into a pre-detach list,\n> > + *         it will be invoked when a device is going to be detached. The\n> > + *         return value will decide if continue detach the device or not.\n> > + *   NULL  lock the device directly, basically this just regiter a empty\n> > + *         callback function(dev_is_busy) that return -EBUSY, so we can\n> > + *         handle the pre-detach check in unified way.\n> > + * @param user_args\n> > + *   parameter will be parsed to callback function, only valid when\n> > + *   callback != NULL.\n> > + * @return\n> > + *   0 on success, negative on error.\n> > + */\n> > +int rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t\n> callback,\n> > +\t\t     void *user_args);\n> \n> I prefer API's that do one thing with one function.\n\nAgree\n\n> Why not\n> \trte_eth_dev_lock(uint16_t port_id);\n> \trte_eth_dev_ondetach(uint16_t port_id, rte_eth_dev_lock_callback_t\n> callback,\n>            \t\t     void *user_args);\n\nRte_eth_dev_ondetach looks like a callback function, \nbut this is the function to register some condition check.\nHow about rte_eth_dev_lock and rte_eth_dev_lock_with_cond?\n\nThanks\nQi",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "dlp-version": "11.0.200.100",
            "dlp-reaction": "no-action",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-6-qi.z.zhang@intel.com>\n\t<20180615090903.67198498@xeon-e3>",
            "X-Mailman-Version": "2.1.15",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,243,1526367600\"; d=\"scan'208\";a=\"64493619\"",
            "From": "\"Zhang, Qi Z\" <qi.z.zhang@intel.com>",
            "x-originating-ip": "[10.239.127.40]",
            "x-titus-metadata-40": "eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTRkY2I2ZDYtZGI1NC00ZTQ1LWI2YWUtZTNmMmNhZjdiZDFlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiME9JQjNUY09FQnpXbmdadjc4T09CNHlGODlSZU9mMHJIXC9OYUJ2ZHBHbXdXWjBsVWNjcU9qSUpBVUljK0ZcL253In0=",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Thread-Index": "AQHUBMM3/7FOR3hvjUSZ2LucRDEGyqRno5SQ",
            "Message-ID": "<039ED4275CED7440929022BC67E706115323A32F@SHSMSX103.ccr.corp.intel.com>",
            "X-ExtLoop1": "1",
            "CC": "\"thomas@monjalon.net\" <thomas@monjalon.net>, \"Burakov, Anatoly\"\n\t<anatoly.burakov@intel.com>, \"Ananyev, Konstantin\"\n\t<konstantin.ananyev@intel.com>, \"dev@dpdk.org\" <dev@dpdk.org>,\n\t\"Richardson, Bruce\" <bruce.richardson@intel.com>, \"Yigit, Ferruh\"\n\t<ferruh.yigit@intel.com>, \"Shelton, Benjamin H\"\n\t<benjamin.h.shelton@intel.com>, \"Vangati, Narender\"\n\t<narender.vangati@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Original-To": "patchwork@dpdk.org",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 5A0244CE4;\n\tTue, 19 Jun 2018 16:16:36 +0200 (CEST)",
                "from mga12.intel.com (mga12.intel.com [192.55.52.136])\n\tby dpdk.org (Postfix) with ESMTP id 5DB1C4CC0\n\tfor <dev@dpdk.org>; Tue, 19 Jun 2018 16:16:35 +0200 (CEST)",
                "from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t19 Jun 2018 07:16:34 -0700",
                "from fmsmsx105.amr.corp.intel.com ([10.18.124.203])\n\tby fmsmga004.fm.intel.com with ESMTP; 19 Jun 2018 07:16:34 -0700",
                "from shsmsx102.ccr.corp.intel.com (10.239.4.154) by\n\tFMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP\n\tServer (TLS) id 14.3.319.2; Tue, 19 Jun 2018 07:16:33 -0700",
                "from shsmsx103.ccr.corp.intel.com ([169.254.4.51]) by\n\tshsmsx102.ccr.corp.intel.com ([169.254.2.223]) with mapi id\n\t14.03.0319.002; Tue, 19 Jun 2018 22:16:31 +0800"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Content-Language": "en-US",
            "Accept-Language": "en-US",
            "Content-Type": "text/plain; charset=\"us-ascii\"",
            "X-MS-Has-Attach": "",
            "X-BeenThere": "dev@dpdk.org",
            "Date": "Tue, 19 Jun 2018 14:16:30 +0000",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "X-Amp-File-Uploaded": "False",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Precedence": "list",
            "Thread-Topic": "[dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
            "In-Reply-To": "<20180615090903.67198498@xeon-e3>",
            "Errors-To": "dev-bounces@dpdk.org",
            "x-ctpclassification": "CTP_NT",
            "dlp-product": "dlpe-windows",
            "MIME-Version": "1.0",
            "Content-Transfer-Encoding": "quoted-printable",
            "To": "Stephen Hemminger <stephen@networkplumber.org>",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "X-MS-TNEF-Correlator": ""
        }
    },
    {
        "id": 82269,
        "web_url": "http://patches.dpdk.org/comment/82269/",
        "msgid": "<20180615090903.67198498@xeon-e3>",
        "date": "2018-06-15T16:09:03",
        "subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
        "submitter": {
            "id": 27,
            "url": "http://patches.dpdk.org/api/people/27/",
            "name": "Stephen Hemminger",
            "email": "stephen@networkplumber.org"
        },
        "content": "On Thu,  7 Jun 2018 20:38:32 +0800\nQi Zhang <qi.z.zhang@intel.com> wrote:\n\n> +/**\n> + * Lock an Ethernet Device directly or register a callback function\n> + * for condition check at runtime, this help application to prevent\n> + * a device be detached unexpectly.\n> + * NOTE: Lock a device mutliple times with same parmeter will increase\n> + * a ref_count, and coresponding unlock decrease the ref_count, the\n> + * device will be unlocked when ref_count reach 0.\n> + *\n> + * @param port_id\n> + *   The port identifier of the Ethernet device.\n> + * @param callback\n> + *   !NULL the callback function will be added into a pre-detach list,\n> + *         it will be invoked when a device is going to be detached. The\n> + *         return value will decide if continue detach the device or not.\n> + *   NULL  lock the device directly, basically this just regiter a empty\n> + *         callback function(dev_is_busy) that return -EBUSY, so we can\n> + *         handle the pre-detach check in unified way.\n> + * @param user_args\n> + *   parameter will be parsed to callback function, only valid when\n> + *   callback != NULL.\n> + * @return\n> + *   0 on success, negative on error.\n> + */\n> +int rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> +\t\t     void *user_args);\n\nI prefer API's that do one thing with one function.\nWhy not\n\trte_eth_dev_lock(uint16_t port_id);\n\trte_eth_dev_ondetach(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n           \t\t     void *user_args);",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-6-qi.z.zhang@intel.com>",
            "X-Mailman-Version": "2.1.15",
            "From": "Stephen Hemminger <stephen@networkplumber.org>",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Content-Type": "text/plain; charset=US-ASCII",
            "X-BeenThere": "dev@dpdk.org",
            "X-Original-To": "patchwork@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:date:from:to:cc:subject:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=GkAaBEBza+sWfVa2d+hNyrLMMZGxLOWbUvWE/c2KaC0=;\n\tb=YqMCXF8zTiZ1V1ARTnbwzkJTl/YkcwGsj8vkTKTnWTSbJENkV66IvxU2tWrIVae++a\n\tgAZqxLorSECENFJOxpOiYcZaQnG2YuGU42Gf+/mMDaMIqnbtZ1DqG+d3siuuRLne1HkU\n\tpyg1LzW4Qr2SH4mJ+qJZ19IpBcun9njUopWbYKsdUgdyQ9SlY5R/YsLe+RvLEUlwmS7T\n\tXzTfJkN/ZBjmjjPS9iXFL2cTjGFRGFmAVqmjVGB2dsEuA6zhGKKJWDppJVru3hrSxCt6\n\t2780hJi8Zjl41JjUc6+Dr8hMQGXgDL8fJ64hFsuZH0RSXRC++oaRprI1rJ9Ha9DjxphN\n\tI2Tg==",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 2E7F01BFE8;\n\tFri, 15 Jun 2018 18:09:09 +0200 (CEST)",
                "from mail-pf0-f195.google.com (mail-pf0-f195.google.com\n\t[209.85.192.195]) by dpdk.org (Postfix) with ESMTP id DC9271BFE7\n\tfor <dev@dpdk.org>; Fri, 15 Jun 2018 18:09:06 +0200 (CEST)",
                "by mail-pf0-f195.google.com with SMTP id a22-v6so5084264pfo.12\n\tfor <dev@dpdk.org>; Fri, 15 Jun 2018 09:09:06 -0700 (PDT)",
                "from xeon-e3 (204-195-35-107.wavecable.com. [204.195.35.107])\n\tby smtp.gmail.com with ESMTPSA id\n\tt5-v6sm12153429pfh.32.2018.06.15.09.09.05\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 15 Jun 2018 09:09:05 -0700 (PDT)"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
            "X-Received": "by 2002:aa7:810c:: with SMTP id\n\tb12-v6mr2589540pfi.79.1529078946001; \n\tFri, 15 Jun 2018 09:09:06 -0700 (PDT)",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=networkplumber-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=GkAaBEBza+sWfVa2d+hNyrLMMZGxLOWbUvWE/c2KaC0=;\n\tb=fMFR17ZleuMy7hLwp45fLGWxac6TV31io8MVkK1/56CdsO7oDANlbvKThrS+6JVak9\n\t6Xqlzv1zaI6pkORf5FF3DI5Ot9H9F7uzHuM6AE7AnSFpXAgVbFh0nGWJYt2Y/FlS2Muz\n\t/AkWimtZQh5MHFGu6duzHHE4NXsZYiiNJUUrDex0cfeYUYITWQwhFyxvCjc0AZBFdZI4\n\t5bsQJ0FOQKdBnMGgDjXAo3MF0vkIodrIEU3YuEBbTbHGsz1hmdNizmJ6jtubb07qsfP7\n\tJ99LgqrW+7M0no5BA1qz6i4biCPodMAfXa696Ixa129DA3ne2hethPq3LYlLZMHbWTL2\n\tpYXw==",
            "Message-ID": "<20180615090903.67198498@xeon-e3>",
            "Precedence": "list",
            "X-Gm-Message-State": "APt69E0cOByyx3EmOCGjJC61mP2PFHXKq+5Aq6H++EmeGlH+KwJMyHst\n\tKq0rln7RGR5ysC//EsqG97anCQ==",
            "Date": "Fri, 15 Jun 2018 09:09:03 -0700",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Errors-To": "dev-bounces@dpdk.org",
            "List-Subscribe": "<https://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "X-Google-Smtp-Source": "ADUXVKIaf/BdU0zmNsQZpdhJqv2Cw3wmtwAbKngpa7KRSKojGAvL/aXgBqtcQCtgkq7upnG8uEx5mg==",
            "Cc": "thomas@monjalon.net, anatoly.burakov@intel.com,\n\tkonstantin.ananyev@intel.com, dev@dpdk.org, bruce.richardson@intel.com,\n\tferruh.yigit@intel.com, benjamin.h.shelton@intel.com,\n\tnarender.vangati@intel.com",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Delivered-To": "patchwork@dpdk.org",
            "In-Reply-To": "<20180607123849.14439-6-qi.z.zhang@intel.com>",
            "List-Unsubscribe": "<https://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "MIME-Version": "1.0",
            "Content-Transfer-Encoding": "7bit",
            "To": "Qi Zhang <qi.z.zhang@intel.com>"
        }
    },
    {
        "id": 82266,
        "web_url": "http://patches.dpdk.org/comment/82266/",
        "msgid": "<bab2b78c-3037-661d-2e18-84e2749f8d94@intel.com>",
        "date": "2018-06-15T15:42:33",
        "subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
        "submitter": {
            "id": 4,
            "url": "http://patches.dpdk.org/api/people/4/",
            "name": "Burakov, Anatoly",
            "email": "anatoly.burakov@intel.com"
        },
        "content": "On 07-Jun-18 1:38 PM, Qi Zhang wrote:\n> Introduce API rte_eth_dev_lock and rte_eth_dev_unlock to let\n> application lock or unlock on specific ethdev, a locked device\n> can't be detached, this help applicaiton to prevent unexpected\n> device detaching, especially in multi-process envrionment.\n> \n> Aslo the new API let application to register a callback function\n> which will be invoked before a device is going to be detached,\n> the return value of the function will decide if device will continue\n> be detached or not, this support application to do condition check\n> at runtime.\n> \n> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>\n> ---\n\n<snip>\n\n> +\n> +int\n> +rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> +\t\t void *user_args)\n> +{\n> +\tRTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);\n> +\n> +\tif (callback == NULL)\n> +\t\treturn register_lock_callback(port_id, dev_is_busy, NULL);\n> +\telse\n> +\t\treturn register_lock_callback(port_id, callback, user_args);\n\nAs much as i don't like seeing negative errno values as return, the rest \nof ethdev library uses those, so this is OK :)\n\n> +}\n> +\n> +int\n> +rte_eth_dev_unlock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> +\t\t   void *user_args)\n> +{\n> +\tRTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);\n> +\n\n<snip>\n\n> + * Also, any callback function return !0 value will prevent device be\n> + * detached(ref. rte_eth_dev_lock and rte_eth_dev_unlock).\n> + *\n> + * @param port_id\n> + *   The port identifier of the Ethernet device.\n> + * @param user_args\n> + *   This is parameter \"user_args\" be saved when callback function is\n> + *   registered(rte_dev_eth_lock).\n> + *\n> + * @return\n> + *   0  device is allowed be detached.\n> + *   !0 device is not allowed be detached.\n\n!0 can be negative or positive. Are we expecting positive return values \nfrom this API?\n\n> + */\n> +typedef int (*rte_eth_dev_lock_callback_t)(uint16_t port_id, void *user_args);\n> +\n> +/**\n> + * Lock an Ethernet Device directly or register a callback function\n> + * for condition check at runtime, this help application to prevent\n> + * a device be detached unexpectly.\n> + * NOTE: Lock a device mutliple times with same parmeter will increase\n> + * a ref_count, and coresponding unlock decrease the ref_count, the\n> + * device will be unlocked when ref_count reach 0.\n\nNitpick: \"note\" sections should be done with @note marker.\n\nAlso, i would mention that this is a per-process lock that does not \naffect other processes (assuming i understood the code correctly, of \ncourse...).\n\n> + *\n> + * @param port_id\n> + *   The port identifier of the Ethernet device.\n> + * @param callback\n> + *   !NULL the callback function will be added into a pre-detach list,\n> + *         it will be invoked when a device is going to be detached. The\n> + *         return value will decide if continue detach the device or not.\n> + *   NULL  lock the device directly, basically this just regiter a empty\n> + *         callback function(dev_is_busy) that return -EBUSY, so we can\n> + *         handle the pre-detach check in unified way.\n> + * @param user_args\n> + *   parameter will be parsed to callback function, only valid when\n> + *   callback != NULL.\n> + * @return\n> + *   0 on success, negative on error.\n> + */\n> +int rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> +\t\t     void *user_args);\n\nNitpicks: DPDK style guide discourages using spaces as indentation \n(other parts of this patch, and other patches have this issue as well).\n\n> +\n> +/**\n> + * Reverse operation of rte_eth_dev_lock.\n> + *\n> + * @param port_id\n> + *   The port identifier of the Ethernet device.\n> + * @param callback\n> + *   NULL  decrease the ref_count of default callback function.\n> + *   !NULL decrease the ref_count of specific callback with matched\n> + *         user_args.\n> + * @param user_args\n> + *   parameter to match, only valid when callback != NULL.\n> + * @return\n> + *   0 on success, negative on error.\n> + */\n> +int rte_eth_dev_unlock(uint16_t port_id, rte_eth_dev_lock_callback_t callback,\n> +\t\t       void *user_args);\n> +\n>   #ifdef __cplusplus\n>   }\n>   #endif\n> diff --git a/lib/librte_ethdev/rte_ethdev_lock.c b/lib/librte_ethdev/rte_ethdev_lock.c\n\nrte_ethdev_lock.* seem to be internal-only files. Perhaps you should \nname them without the rte_ prefix to indicate that they're not exported?\n\n> new file mode 100644\n> index 000000000..688d1d70a\n> --- /dev/null\n> +++ b/lib/librte_ethdev/rte_ethdev_lock.c\n> @@ -0,0 +1,102 @@\n> +/* SPDX-License-Identifier: BSD-3-Clause\n> + * Copyright(c) 2018 Intel Corporation\n> + */\n> +#include \"rte_ethdev_lock.h\"\n> +\n> +struct lock_entry {\n> +\tTAILQ_ENTRY(lock_entry) next;\n> +\trte_eth_dev_lock_callback_t callback;\n> +\tuint16_t port_id;\n\n<snip>\n\n> +register_lock_callback(uint16_t port_id,\n> +\t\t       rte_eth_dev_lock_callback_t callback,\n> +\t\t       void *user_args)\n> +{\n> +\tstruct lock_entry *le;\n> +\n> +\trte_spinlock_lock(&lock_entry_lock);\n> +\n> +\tTAILQ_FOREACH(le, &lock_entry_list, next) {\n> +\t\tif (le->port_id == port_id &&\n> +\t\t    le->callback == callback &&\n> +\t\t    le->user_args == user_args)\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\tif (!le) {\n> +\t\tle = calloc(1, sizeof(struct lock_entry));\n> +\t\tif (!le) {\n\nNitpick: generally, DPDK style guide prefers \"if (value)\" or \"if \n(!value)\" to only be reserved for boolean values, and use explicit \ncomparison (e.g. \"if (value == NULL)\" or \"if (value == 0)\") for all \nother cases.",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-6-qi.z.zhang@intel.com>",
            "X-Mailman-Version": "2.1.15",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,227,1526367600\"; d=\"scan'208\";a=\"57630072\"",
            "From": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>",
            "User-Agent": "Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.8.0",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Content-Type": "text/plain; charset=utf-8; format=flowed",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Original-To": "patchwork@dpdk.org",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id AB2731BEB6;\n\tFri, 15 Jun 2018 17:42:39 +0200 (CEST)",
                "from mga12.intel.com (mga12.intel.com [192.55.52.136])\n\tby dpdk.org (Postfix) with ESMTP id D58AF1BEB5\n\tfor <dev@dpdk.org>; Fri, 15 Jun 2018 17:42:36 +0200 (CEST)",
                "from fmsmga003.fm.intel.com ([10.253.24.29])\n\tby fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t15 Jun 2018 08:42:35 -0700",
                "from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.66])\n\t([10.237.220.66])\n\tby FMSMGA003.fm.intel.com with ESMTP; 15 Jun 2018 08:42:34 -0700"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "X-Amp-File-Uploaded": "False",
            "Content-Language": "en-US",
            "Message-ID": "<bab2b78c-3037-661d-2e18-84e2749f8d94@intel.com>",
            "Precedence": "list",
            "X-BeenThere": "dev@dpdk.org",
            "Date": "Fri, 15 Jun 2018 16:42:33 +0100",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "X-ExtLoop1": "1",
            "List-Subscribe": "<https://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Cc": "konstantin.ananyev@intel.com, dev@dpdk.org, bruce.richardson@intel.com, \n\tferruh.yigit@intel.com, benjamin.h.shelton@intel.com,\n\tnarender.vangati@intel.com",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "In-Reply-To": "<20180607123849.14439-6-qi.z.zhang@intel.com>",
            "List-Unsubscribe": "<https://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "MIME-Version": "1.0",
            "Content-Transfer-Encoding": "7bit",
            "To": "Qi Zhang <qi.z.zhang@intel.com>, thomas@monjalon.net",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "Errors-To": "dev-bounces@dpdk.org"
        }
    }
]