[{"id":1764219,"web_url":"http://patchwork.ozlabs.org/comment/1764219/","msgid":"","list_archive_url":null,"date":"2017-09-06T15:58:16","subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","submitter":{"id":65180,"url":"http://patchwork.ozlabs.org/api/people/65180/","name":"Santosh Shilimkar","email":"santosh.shilimkar@oracle.com"},"content":"On 9/6/2017 8:29 AM, Håkon Bugge wrote:\n> In rds_send_xmit() there is logic to batch the sends. However, if\n> another thread has acquired the lock, it is considered a race and we\n> yield. The code incrementing the s_send_lock_queue_raced statistics\n> counter did not count this event correctly.\n> \n> This commit removes a small race in determining the race and\n> increments the statistics counter correctly.\n> \n> Signed-off-by: Håkon Bugge \n> Reviewed-by: Knut Omang \n> ---\n> net/rds/send.c | 16 +++++++++++++---\n> 1 file changed, 13 insertions(+), 3 deletions(-)\n> \nThose counters are not really to give that accurate so\nam not very keen to add additional cycles in send paths\nand add additional code. Have you seen any real issue\nor this is just a observation. s_send_lock_queue_raced\ncounter is never used to check for smaller increments\nand hence the question.\n\nRegards,\nSantosh","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnStG05VBz9t4c\n\tfor ;\n\tThu, 7 Sep 2017 01:58:30 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755174AbdIFP6O (ORCPT );\n\tWed, 6 Sep 2017 11:58:14 -0400","from aserp1040.oracle.com ([141.146.126.69]:40267 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754372AbdIFP6N (ORCPT\n\t); Wed, 6 Sep 2017 11:58:13 -0400","from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2)\n\twith ESMTP id v86FvdtA008022\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 15:57:40 GMT","from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72])\n\tby aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v86FvcvP024143\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 15:57:39 GMT","from abhmp0004.oracle.com (abhmp0004.oracle.com [141.146.116.10])\n\tby userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id\n\tv86Fvcv0014119; Wed, 6 Sep 2017 15:57:38 GMT","from [10.209.243.219] (/10.209.243.219)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Wed, 06 Sep 2017 08:57:37 -0700"],"Subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","To":"=?utf-8?q?H=C3=A5kon_Bugge?= ,\n\t\"David S . Miller\" ","Cc":"netdev@vger.kernel.org, linux-rdma@vger.kernel.org,\n\trds-devel@oss.oracle.com, linux-kernel@vger.kernel.org,\n\tknut.omang@oracle.com","References":"<20170906152950.17766-1-Haakon.Bugge@oracle.com>","From":"Santosh Shilimkar ","Organization":"Oracle Corporation","Message-ID":"","Date":"Wed, 6 Sep 2017 08:58:16 -0700","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170906152950.17766-1-Haakon.Bugge@oracle.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-Source-IP":"aserv0022.oracle.com [141.146.126.234]","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1764228,"web_url":"http://patchwork.ozlabs.org/comment/1764228/","msgid":"<715EA84D-6ACA-45DE-9EA2-6122E11545E8@oracle.com>","list_archive_url":null,"date":"2017-09-06T16:12:58","subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","submitter":{"id":72009,"url":"http://patchwork.ozlabs.org/api/people/72009/","name":"Haakon Bugge","email":"Haakon.Bugge@oracle.com"},"content":"> On 6 Sep 2017, at 17:58, Santosh Shilimkar wrote:\n> \n> On 9/6/2017 8:29 AM, Håkon Bugge wrote:\n>> In rds_send_xmit() there is logic to batch the sends. However, if\n>> another thread has acquired the lock, it is considered a race and we\n>> yield. The code incrementing the s_send_lock_queue_raced statistics\n>> counter did not count this event correctly.\n>> This commit removes a small race in determining the race and\n>> increments the statistics counter correctly.\n>> Signed-off-by: Håkon Bugge \n>> Reviewed-by: Knut Omang \n>> ---\n>> net/rds/send.c | 16 +++++++++++++---\n>> 1 file changed, 13 insertions(+), 3 deletions(-)\n> Those counters are not really to give that accurate so\n> am not very keen to add additional cycles in send paths\n> and add additional code. Have you seen any real issue\n> or this is just a observation. s_send_lock_queue_raced\n> counter is never used to check for smaller increments\n> and hence the question.\n\nHi Santosh,\n\n\nYes, I agree with accuracy of s_send_lock_queue_raced. But the main point is that the existing code counts some partial share of when it is _not_ raced.\n\nSo, in the critical path, my patch adds one test_bit(), which hits the local CPU cache, if not raced. If raced, some other thread is in control, so I would not think the added cycles would make any big difference.\n\nI can send a v2 where the race tightening is removed if you like.\n\n\nThxs, Håkon","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnTCx3d7pz9s7c\n\tfor ;\n\tThu, 7 Sep 2017 02:13:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755302AbdIFQNh convert rfc822-to-8bit (ORCPT\n\t);\n\tWed, 6 Sep 2017 12:13:37 -0400","from aserp1040.oracle.com ([141.146.126.69]:17708 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751365AbdIFQNf (ORCPT\n\t); Wed, 6 Sep 2017 12:13:35 -0400","from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v86GD2nK000760\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 16:13:02 GMT","from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72])\n\tby userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v86GD1f4029440\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 16:13:01 GMT","from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16])\n\tby userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id\n\tv86GD1lA024578; Wed, 6 Sep 2017 16:13:01 GMT","from dhcp-ukc1-twvpn-3-vpnpool-10-175-254-104.vpn.oracle.com\n\t(/10.175.254.104) by default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Wed, 06 Sep 2017 09:13:01 -0700"],"Content-Type":"text/plain; charset=utf-8","Mime-Version":"1.0 (Mac OS X Mail 10.3 \\(3273\\))","Subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","From":"=?utf-8?q?H=C3=A5kon_Bugge?= ","In-Reply-To":"","Date":"Wed, 6 Sep 2017 18:12:58 +0200","Cc":"\"David S . Miller\" , netdev@vger.kernel.org,\n\tOFED mailing list ,\n\trds-devel@oss.oracle.com, linux-kernel@vger.kernel.org,\n\tKnut Omang ","Content-Transfer-Encoding":"8BIT","Message-Id":"<715EA84D-6ACA-45DE-9EA2-6122E11545E8@oracle.com>","References":"<20170906152950.17766-1-Haakon.Bugge@oracle.com>\n\t","To":"Santosh Shilimkar ","X-Mailer":"Apple Mail (2.3273)","X-Source-IP":"userv0021.oracle.com [156.151.31.71]","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1764237,"web_url":"http://patchwork.ozlabs.org/comment/1764237/","msgid":"<6fb62a09-13b4-0cec-2864-40afdfc51d25@oracle.com>","list_archive_url":null,"date":"2017-09-06T16:21:01","subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","submitter":{"id":65180,"url":"http://patchwork.ozlabs.org/api/people/65180/","name":"Santosh Shilimkar","email":"santosh.shilimkar@oracle.com"},"content":"On 9/6/2017 9:12 AM, Håkon Bugge wrote:\n> \n[...]\n\n> \n> Hi Santosh,\n> \n> \n> Yes, I agree with accuracy of s_send_lock_queue_raced. But the main point is that the existing code counts some partial share of when it is _not_ raced.\n> \n> So, in the critical path, my patch adds one test_bit(), which hits the local CPU cache, if not raced. If raced, some other thread is in control, so I would not think the added cycles would make any big difference.\n>\nCycles added for no good reason is the point.\n\n> I can send a v2 where the race tightening is removed if you like.\n> \nYes please.\n\nRegards,\nSantosh","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnTNS0S93z9s7F\n\tfor ;\n\tThu, 7 Sep 2017 02:21:12 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755390AbdIFQU7 (ORCPT );\n\tWed, 6 Sep 2017 12:20:59 -0400","from aserp1040.oracle.com ([141.146.126.69]:23734 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752462AbdIFQU5 (ORCPT\n\t); Wed, 6 Sep 2017 12:20:57 -0400","from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v86GKPJr012423\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 16:20:25 GMT","from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235])\n\tby userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv86GKOBC004968\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Wed, 6 Sep 2017 16:20:24 GMT","from abhmp0017.oracle.com (abhmp0017.oracle.com [141.146.116.23])\n\tby aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id\n\tv86GKOkF009997; Wed, 6 Sep 2017 16:20:24 GMT","from [10.209.243.219] (/10.209.243.219)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Wed, 06 Sep 2017 09:20:24 -0700"],"Subject":"Re: [PATCH net] rds: Fix incorrect statistics counting","To":"=?utf-8?q?H=C3=A5kon_Bugge?= ","Cc":"\"David S . Miller\" , netdev@vger.kernel.org,\n\tOFED mailing list ,\n\trds-devel@oss.oracle.com, linux-kernel@vger.kernel.org,\n\tKnut Omang ","References":"<20170906152950.17766-1-Haakon.Bugge@oracle.com>\n\t\n\t<715EA84D-6ACA-45DE-9EA2-6122E11545E8@oracle.com>","From":"Santosh Shilimkar ","Organization":"Oracle Corporation","Message-ID":"<6fb62a09-13b4-0cec-2864-40afdfc51d25@oracle.com>","Date":"Wed, 6 Sep 2017 09:21:01 -0700","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<715EA84D-6ACA-45DE-9EA2-6122E11545E8@oracle.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-Source-IP":"userv0022.oracle.com [156.151.31.74]","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]