[{"id":1763544,"web_url":"http://patchwork.ozlabs.org/comment/1763544/","msgid":"","list_archive_url":null,"date":"2017-09-05T18:07:37","subject":"Re: [PATCH] radix-tree: must check __radix_tree_preload() return\n\tvalue","submitter":{"id":97,"url":"http://patchwork.ozlabs.org/api/people/97/","name":"Linus Torvalds","email":"torvalds@linux-foundation.org"},"content":"On Tue, Sep 5, 2017 at 10:59 AM, Eric Dumazet wrote:\n> @@ -2104,7 +2104,10 @@ EXPORT_SYMBOL(radix_tree_tagged);\n> */\n> void idr_preload(gfp_t gfp_mask)\n> {\n> - __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);\n> + int ret = __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);\n> +\n> + if (ret)\n> + preempt_disable();\n> }\n> EXPORT_SYMBOL(idr_preload);\n\nIs there a reason for the \"ret\" variable that is entirely mis-named,\nsince it's never actually used as a return value?\n\n(Sure. it's the return value of a function, but that is entirely\nuseless and pointless information, and adds no value. We should name\nvariables by the data they contain or how they are used, not by \"it\nwas the return value of a function\").\n\nIn other words, why isn't this just\n\n if (__radix_tree_preload(..))\n preempt_disable();\n\nwhich is shorter and clearer and not confusing?\n\n> @@ -2118,13 +2121,14 @@ EXPORT_SYMBOL(idr_preload);\n> */\n> int ida_pre_get(struct ida *ida, gfp_t gfp)\n> {\n> - __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);\n> + int ret = __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);\n> /*\n> * The IDA API has no preload_end() equivalent. Instead,\n> * ida_get_new() can return -EAGAIN, prompting the caller\n> * to return to the ida_pre_get() step.\n> */\n> - preempt_enable();\n> + if (!ret)\n> + preempt_enable();\n\nSame issue, but this time strengthened by an additional \"why doesn't\nthis just use that idr_preload function then?\" question..\n\n Linus","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=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"uJkBg4I4\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmvp43pWQz9sNr\n\tfor ;\n\tWed, 6 Sep 2017 04:07:56 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753269AbdIESHk (ORCPT );\n\tTue, 5 Sep 2017 14:07:40 -0400","from mail-io0-f196.google.com ([209.85.223.196]:38101 \"EHLO\n\tmail-io0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752175AbdIESHi (ORCPT\n\t); Tue, 5 Sep 2017 14:07:38 -0400","by mail-io0-f196.google.com with SMTP id q64so3583468iod.5;\n\tTue, 05 Sep 2017 11:07:38 -0700 (PDT)","by 10.107.15.151 with HTTP; Tue, 5 Sep 2017 11:07:37 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=tYY5vRU48+GiTAjSnEkbaNF48dhLV2JiSD9rETKj/aI=;\n\tb=uJkBg4I45CtmFGE5yCrdoOrb+W214IfBF1rXGJ2tJRtIt4VVEK80SVcpkc8wEJKxnr\n\t1k9JWeqhUnErOia/GgeQX7EkMI2ciFp6spO5ZqyNvCXw3vQNoF2Eg8InaYUSF7MJdRgZ\n\tDtEeoMtcjlENtnV81PFb94n/60ZbdL6FTSVPhPkV6/nGun+bSVVqhknJriKIBrdqd5hh\n\tNTGwyBDynjce/2RgYB+CH2iQNnWlUR0LK/GLhkSj8ZjPeEj6pe9GIjiwddkN3pnB6SBr\n\tJK2JhAj4/8UlTrjrAS7fYiArPcJ9ZHJ5yfxAwxbT61zCqPFH5nRTt4CXBvTmYCiGeo4N\n\t8PMA==","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:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=tYY5vRU48+GiTAjSnEkbaNF48dhLV2JiSD9rETKj/aI=;\n\tb=DyEoGyrFNuy/2b99BeEgXa/IrpTgbdlIceBrWqL1NfEtM8dU5Qrk3F+W3tgHtuUEk0\n\tQEIH2rM2JWm80SpVxC4oBXuJ5EbNGn9TQuqrWOx5MWibez2W5ORnxhuBTsdnBIw4OZry\n\tQXDQ2fotv8yzEwfSq09a0Pt/NIIpd9m+dJCVaEuyr9tdvLIFoC1Rbt+kvV7tx6bA9+4o\n\tNLnyHAuzVipG0a2nsT7lOQ0T3mI4D2uSMEXdDk0GnWsSFI+ETqp/Q4q9evc+MqZ0qHrv\n\tC8yDrWO3klmLABAzLoOcSZ0KxbbZBxIdGyHxRxSljLTe4+85kK9TPu3ce8jMn2vb+ibP\n\tDsDA==","X-Gm-Message-State":"AHPjjUiUp8TGMBW5tZ2wuAsPwiSug7DcH86Pi0b2U7SYZCfI3TtbXJe3\n\tWIFbDJIeudyoV/okyg31l1s7wP449w==","X-Google-Smtp-Source":"ADKCNb6AS0o+7sGhkWHG3BtDJFhzHavNFRsO/NaKGvJ7FWo4w+jqf+5g5v/q2WThO636FDI8CxGfaub814pfXdJN6vQ=","X-Received":"by 10.107.169.91 with SMTP id s88mr4824262ioe.179.1504634857824; \n\tTue, 05 Sep 2017 11:07:37 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1504634367.15310.59.camel@edumazet-glaptop3.roam.corp.google.com>","References":"<1504634367.15310.59.camel@edumazet-glaptop3.roam.corp.google.com>","From":"Linus Torvalds ","Date":"Tue, 5 Sep 2017 11:07:37 -0700","X-Google-Sender-Auth":"wGbDD2gXLWJbwCG5o3dGiB9lWGc","Message-ID":"","Subject":"Re: [PATCH] radix-tree: must check __radix_tree_preload() return\n\tvalue","To":"Eric Dumazet ","Cc":"Matthew Wilcox ,\n\tMatthew Wilcox ,\n\t\"Kirill A. Shutemov\" ,\n\tAndrew Morton ,\n\tnetdev ,\n\tlinux-kernel ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1763558,"web_url":"http://patchwork.ozlabs.org/comment/1763558/","msgid":"<1504636475.15310.60.camel@edumazet-glaptop3.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-05T18:34:35","subject":"Re: [PATCH] radix-tree: must check __radix_tree_preload() return\n\tvalue","submitter":{"id":2404,"url":"http://patchwork.ozlabs.org/api/people/2404/","name":"Eric Dumazet","email":"eric.dumazet@gmail.com"},"content":"On Tue, 2017-09-05 at 11:07 -0700, Linus Torvalds wrote:\n> On Tue, Sep 5, 2017 at 10:59 AM, Eric Dumazet wrote:\n> > @@ -2104,7 +2104,10 @@ EXPORT_SYMBOL(radix_tree_tagged);\n> > */\n> > void idr_preload(gfp_t gfp_mask)\n> > {\n> > - __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);\n> > + int ret = __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);\n> > +\n> > + if (ret)\n> > + preempt_disable();\n> > }\n> > EXPORT_SYMBOL(idr_preload);\n> \n> Is there a reason for the \"ret\" variable that is entirely mis-named,\n> since it's never actually used as a return value?\n> \n> (Sure. it's the return value of a function, but that is entirely\n> useless and pointless information, and adds no value. We should name\n> variables by the data they contain or how they are used, not by \"it\n> was the return value of a function\").\n> \n> In other words, why isn't this just\n> \n> if (__radix_tree_preload(..))\n> preempt_disable();\n> \n> which is shorter and clearer and not confusing?\n\n\n...\n\n> Same issue, but this time strengthened by an additional \"why doesn't\n> this just use that idr_preload function then?\" question..\n> \n\nYep, I will send a v2, thanks.","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"P4odn5a6\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmwPl23wBz9sCZ\n\tfor ;\n\tWed, 6 Sep 2017 04:35:23 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753158AbdIESeu (ORCPT );\n\tTue, 5 Sep 2017 14:34:50 -0400","from mail-pf0-f194.google.com ([209.85.192.194]:37912 \"EHLO\n\tmail-pf0-f194.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752766AbdIESeh (ORCPT\n\t); Tue, 5 Sep 2017 14:34:37 -0400","by mail-pf0-f194.google.com with SMTP id q76so2010536pfq.5;\n\tTue, 05 Sep 2017 11:34:37 -0700 (PDT)","from ?IPv6:2620:15c:2c1:100:71d2:2db4:a10e:204e?\n\t([2620:15c:2c1:100:71d2:2db4:a10e:204e])\n\tby smtp.googlemail.com with ESMTPSA id\n\td12sm2080195pgn.53.2017.09.05.11.34.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tTue, 05 Sep 2017 11:34:36 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=S9nGDPoyvnvTPhSNO690m89RPs52jsBeKJqsm5cJ/RM=;\n\tb=P4odn5a6qapV2PEGVHkHoTZppGqe38IE6F0OPkr8MRxDU/Gs6MIkL3ZNaN79X8X1kT\n\tfNyYHCc1HAv87jB5RiOKeVKBXYBjotx6lc9NMg7PaRwV/6sDjwOh9X7iEK9//BViHcoT\n\tDwhUSS2dBkyJzPXzJk37w1Me/iTeBVPnLVDVPnznEiJ8s7NVRSw5ADOe/f6sT1e1LlOd\n\tr10SGZS+arpzRUB1P5NcXVYLhqGKIdPFgv+jzHURqHOg+pE95rLq0KzDMOKJJ4RV9Y0X\n\tSKSOOxfgr+C/suE5MxOfiZ989CSCesNUpHrfUK78hlPKWTtRpURZHTd6Od57WIg1gh/d\n\tTDkA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=S9nGDPoyvnvTPhSNO690m89RPs52jsBeKJqsm5cJ/RM=;\n\tb=o2xK//uvQ6/NkzQeZtKh48CngKqD0sGfdQ7pDK1kkMLcQODrSTcy0vsNJ9zRuCHAi3\n\tH/hLEv8/QYRO/Ks+GlwMgUgF4/271YJDPnjMWR3sVyDCMQgiRmdrOyYjrkXb0umiZw8B\n\tNluinfcrT6zuz1I0iD45w0+CLBReDs68D4GmtsdD+JArgFKU/iX4Hk7aO36MLBCx/0ij\n\t4i0DwwUuboG0vIiMDCQ+2laRJCJrxUhIhDfenyqrKWNesNUZsYUBVUjSYyOoUQrfwySA\n\t7puYAJteSDjNMGg8EqL0ry6hcrHcSrf9sTssRF2i7TlBMWCXIwTyJzO/yumHj/i6hFL8\n\t2rSQ==","X-Gm-Message-State":"AHPjjUimtJhDzKjxu+5GCi2wlR+k79M0FvpQ3PLlMR8IRiUFZTcnZUPD\n\t/WGtU0Kd/WDv1g==","X-Google-Smtp-Source":"ADKCNb5ygRh+LhQCcK0iTk2800Qg701TVD7WJmqq/UGAPcmruoQEut0nPrenq2zMFZnHgdt0IPRSaw==","X-Received":"by 10.98.64.138 with SMTP id f10mr4664938pfd.246.1504636477351; \n\tTue, 05 Sep 2017 11:34:37 -0700 (PDT)","Message-ID":"<1504636475.15310.60.camel@edumazet-glaptop3.roam.corp.google.com>","Subject":"Re: [PATCH] radix-tree: must check __radix_tree_preload() return\n\tvalue","From":"Eric Dumazet ","To":"Linus Torvalds ","Cc":"Matthew Wilcox ,\n\tMatthew Wilcox ,\n\t\"Kirill A. Shutemov\" ,\n\tAndrew Morton ,\n\tnetdev ,\n\tlinux-kernel ","Date":"Tue, 05 Sep 2017 11:34:35 -0700","In-Reply-To":"","References":"<1504634367.15310.59.camel@edumazet-glaptop3.roam.corp.google.com>\n\t","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.10.4-0ubuntu2 ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]