From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932117AbbBZAN0 (ORCPT ); Wed, 25 Feb 2015 19:13:26 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:42766 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932079AbbBZANX (ORCPT ); Wed, 25 Feb 2015 19:13:23 -0500 Date: Wed, 25 Feb 2015 16:13:13 -0800 From: Guenter Roeck To: Bartosz Golaszewski Cc: Phil Pokorny , LKML , lm-sensors Subject: Re: [lm-sensors] [PATCH 1/4] kernel.h: add find_closest() macro Message-ID: <20150226001313.GA14592@roeck-us.net> References: <1424799734-2170-1-git-send-email-bgolaszewski@baylibre.com> <1424799734-2170-2-git-send-email-bgolaszewski@baylibre.com> <20150224205125.GB18025@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-CTCH-PVer: 0000001 X-CTCH-Spam: Unknown X-CTCH-VOD: Unknown X-CTCH-Flags: 0 X-CTCH-RefID: str=0001.0A020204.54EE6522.0115,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 X-CTCH-Score: 0.001 X-CTCH-ScoreCust: 0.000 X-CTCH-Rules: C_4847, X-CTCH-SenderID: linux@roeck-us.net X-CTCH-SenderID-Flags: 0 X-CTCH-SenderID-TotalMessages: 3 X-CTCH-SenderID-TotalSpam: 0 X-CTCH-SenderID-TotalSuspected: 0 X-CTCH-SenderID-TotalConfirmed: 0 X-CTCH-SenderID-TotalBulk: 0 X-CTCH-SenderID-TotalVirus: 0 X-CTCH-SenderID-TotalRecipients: 0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: mailgid no entry from get_relayhosts_entry X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 25, 2015 at 11:59:51AM +0100, Bartosz Golaszewski wrote: > 2015-02-24 21:51 GMT+01:00 Guenter Roeck : > > I think the lm85 conversion actually introduces a bug with such an > > off-by-one mistake. And if it doesn't, there is still a unexplained > > and not easy to understand '-1' in one of the calls to find_closest(). > > > > So the question is if the new code really improves the situation in that > > respect. > > Yes, it's a mistake. I couldn't test this one and missed this '-1'. > Sorry for that. > > As for the size comparisons - at first glance it seems as if it adds bloat: > > ina2xx: > add/remove: 0/0 grow/shrink: 1/0 up/down: 24/0 (24) > function old new delta > ina226_set_interval 296 320 +24 > > lm85: > add/remove: 0/0 grow/shrink: 3/0 up/down: 72/0 (72) > function old new delta > set_temp_auto_temp_min 364 388 +24 > set_temp_auto_temp_max 336 360 +24 > set_pwm_freq 284 308 +24 > > w83795: > add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0 (16) > function old new delta > store_pwm 496 512 +16 > > But this actually comes from DIV_ROUND_CLOSEST() since replacing it > with a simple '/ 2' gives different results: > > ina2xx.ko: > add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6) > function old new delta > __UNIQUE_ID_vermagic0 73 79 +6 > > lm85.ko: > add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6) > function old new delta > __UNIQUE_ID_vermagic0 73 79 +6 > > w83795.ko: > add/remove: 0/0 grow/shrink: 2/0 up/down: 14/0 (14) > function old new delta > store_pwm 496 504 +8 > __UNIQUE_ID_vermagic0 73 79 +6 > > The idea however was to remove duplicated operations from source files > and prevent off-by-one bugs (how ironic the lm85 patch...), not really > to reduce size. > I don't know, seems to me it can cause more trouble and potential for getting something wrong than it is worth. I'll definitely want to see feedback (and acceptance) from more senior maintainers than myself. Guenter