Message ID | 20230616142614.36206-1-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp1389092vqr; Fri, 16 Jun 2023 07:36:05 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4T+dQhoYXBoIFpjbbwSjKCzCpL6HmnQ9zDNPgtpdBCMVi3jJrj/3y6hZ2Vu7ezbKywOqea X-Received: by 2002:a17:903:4d5:b0:1b5:391f:7636 with SMTP id jm21-20020a17090304d500b001b5391f7636mr904609plb.39.1686926164836; Fri, 16 Jun 2023 07:36:04 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686926164; cv=none; d=google.com; s=arc-20160816; b=mSsC7viXpXWM6Yush4rgkrK2nqR2BBNHvTn9mal3bRdD9KGumL5nSa92q6tJs5BU/+ ZZgWIFWwEaGBlcMOXE2VPLhZOXFQJg+s34y2cS/v9kxqN95baKTj38pZ0vMgng/oVps7 d1HQKUijSncGnQI8sMds1XDmsUMjhuHgXozdLA79aQjmuh1HS6o11vkVJPWIVOv72Ulo tB4wCRMkNlk8mHuotZk4gormIhZMTlWGgeAZT0Gnmp/+q3VJFKPmb4ZofaSPhz2wJCWv TpXTctPPiGn43mtZoGKBqE1lJk63xPR8Rf5eTfq+SIwP1h+GeC5cE0rF8k6A6VZMsMeP oZLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=Ahh/fsHN0q/hwtYc32l43F0+Yd52MfDa7q87AdE5jk0=; b=OyqBrxQfEJQGR3Se6MvSKLwVcgg1krZAcJtYsF71sZHdfx97V1//DhmdvbIoMY+2/0 leBJNB0nspBUVdIGgzPRBxOAQ6HCuyLobQHC0eMyeYxCDrgS7b/c3XtlMeUOd8BNnhMU O7rTnfBhFC0G5IBOuRO7eNsv7qm5vOgRslzRZ/xg/HLXOe2XvYlyMd0SZjoocFRGQ5Oz VustlYSq2jtWqwHeEn6FBtadhFRw++fikamPv9Tl5o6O6MZKk4BevTkZdLgkF0Myanjc JFTNZYLicrAS9JfVhFQRd/d1Twtfi3ozB5CjVJUbz8aaw2myCnkTRD0ljPBxXEK4+xhu auRg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=lPYbMMdP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a21-20020a170902b59500b001ac83d19265si14467894pls.291.2023.06.16.07.35.49; Fri, 16 Jun 2023 07:36:04 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=lPYbMMdP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345493AbjFPO0P (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Fri, 16 Jun 2023 10:26:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39342 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345434AbjFPO0N (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 16 Jun 2023 10:26:13 -0400 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02BC230E3; Fri, 16 Jun 2023 07:26:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686925571; x=1718461571; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=jbvx+fzCk6wuoZXDOskZS3VFaadcPZdfZkhrwY1ULLc=; b=lPYbMMdPn16dDXSSud8doBGuw+4pB1fxXcWMrTmIYmPVIIasgccFYdRN W2r0LAgibmZvQ9eZar0E0RSFBn+4oipgbfXsLppaLJpzL5DPiG4EGbKrK krZ2nlO1UXjMxJ2mpyHRc3mQKrydzAuz6p46Eb4xBfWaElBfJbYBqIwdl wefvLozWVDvAgFNd3m0tKYxZ1TyiGXrz7AEp/0FYDnD/0DZAn+y0vxknq dT3sIiOu3Nv1wL/gGP7i/LCnblQvka8B92UDiqYKiG5GF+DKRwCZdAOqm S1Xvg/apXfCKdUI0ELZ9fPhbmCMwENnYZuoiTARIkESOB96Q+fPVNOvQT Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10742"; a="348934347" X-IronPort-AV: E=Sophos;i="6.00,247,1681196400"; d="scan'208";a="348934347" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jun 2023 07:26:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10742"; a="716043859" X-IronPort-AV: E=Sophos;i="6.00,247,1681196400"; d="scan'208";a="716043859" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga007.fm.intel.com with ESMTP; 16 Jun 2023 07:26:08 -0700 Received: by black.fi.intel.com (Postfix, from userid 1003) id BC90E379; Fri, 16 Jun 2023 17:26:17 +0300 (EEST) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Jens Axboe <axboe@kernel.dk>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Subject: [PATCH v1 1/1] pktcdvd: Use clamp_val() instead of min()+max() Date: Fri, 16 Jun 2023 17:26:14 +0300 Message-Id: <20230616142614.36206-1-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.40.0.1.gaa8946217a0b MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768870290086427040?= X-GMAIL-MSGID: =?utf-8?q?1768870290086427040?= |
Series |
[v1,1/1] pktcdvd: Use clamp_val() instead of min()+max()
|
|
Commit Message
Andy Shevchenko
June 16, 2023, 2:26 p.m. UTC
In a couple of places replace min()+max() pair by clamp_val().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/block/pktcdvd.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
Comments
From: Andy Shevchenko > Sent: 16 June 2023 15:26 > > In a couple of places replace min()+max() pair by clamp_val(). > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/block/pktcdvd.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c > index a1428538bda5..18a960bb6165 100644 > --- a/drivers/block/pktcdvd.c > +++ b/drivers/block/pktcdvd.c > @@ -208,14 +208,11 @@ static DEVICE_ATTR_RO(size); > static void init_write_congestion_marks(int* lo, int* hi) > { > if (*hi > 0) { > - *hi = max(*hi, 500); > - *hi = min(*hi, 1000000); > + *hi = clamp_val(*hi, 500, 1000000); (standard rant about minmax.h) clamp_val() is pretty much broken by design. It MIGHT be ok here but it casts both limits to the type of the value being compared. In general that is just plain wrong. Like min_t() it is generally ok because the kernel only uses unsigned values between 0 and MAXINT. If min/max were ok, then using clamp() should also be ok. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, Jun 20, 2023 at 12:06:49PM +0000, David Laight wrote: ... > > + *hi = clamp_val(*hi, 500, 1000000); > > (standard rant about minmax.h) > > clamp_val() is pretty much broken by design. > It MIGHT be ok here but it casts both limits to the > type of the value being compared. > In general that is just plain wrong. > > Like min_t() it is generally ok because the kernel only uses > unsigned values between 0 and MAXINT. > > If min/max were ok, then using clamp() should also be ok. Submit a patch to fix it, if you think you can make it better. Obviously your comment can be addressed separately if we even need that.
From: 'Andy Shevchenko' > Sent: 20 June 2023 14:25 > > On Tue, Jun 20, 2023 at 12:06:49PM +0000, David Laight wrote: > > ... > > > > + *hi = clamp_val(*hi, 500, 1000000); > > > > (standard rant about minmax.h) > > > > clamp_val() is pretty much broken by design. > > It MIGHT be ok here but it casts both limits to the > > type of the value being compared. > > In general that is just plain wrong. > > > > Like min_t() it is generally ok because the kernel only uses > > unsigned values between 0 and MAXINT. > > > > If min/max were ok, then using clamp() should also be ok. > > Submit a patch to fix it, if you think you can make it better. > Obviously your comment can be addressed separately if we even > need that. Did you try using clamp() ? To see why clamp_val() is broken consider? unsigned char val = 200; ... xxx = clamp_val(val, 10, 300); This sets xxx to 44 - not exactly expected. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, Jun 20, 2023 at 01:35:11PM +0000, David Laight wrote: > From: 'Andy Shevchenko' > > Sent: 20 June 2023 14:25 > > On Tue, Jun 20, 2023 at 12:06:49PM +0000, David Laight wrote: ... > > > > + *hi = clamp_val(*hi, 500, 1000000); > > > > > > (standard rant about minmax.h) > > > > > > clamp_val() is pretty much broken by design. > > > It MIGHT be ok here but it casts both limits to the > > > type of the value being compared. > > > In general that is just plain wrong. > > > > > > Like min_t() it is generally ok because the kernel only uses > > > unsigned values between 0 and MAXINT. > > > > > > If min/max were ok, then using clamp() should also be ok. > > > > Submit a patch to fix it, if you think you can make it better. > > Obviously your comment can be addressed separately if we even > > need that. > > Did you try using clamp() ? > > To see why clamp_val() is broken consider? > unsigned char val = 200; > ... > xxx = clamp_val(val, 10, 300); > > This sets xxx to 44 - not exactly expected. Right, clamp_val() has to be improved. However this is not the case in this driver. I have just sent a v2 with clamp().
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index a1428538bda5..18a960bb6165 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -208,14 +208,11 @@ static DEVICE_ATTR_RO(size); static void init_write_congestion_marks(int* lo, int* hi) { if (*hi > 0) { - *hi = max(*hi, 500); - *hi = min(*hi, 1000000); + *hi = clamp_val(*hi, 500, 1000000); if (*lo <= 0) *lo = *hi - 100; - else { - *lo = min(*lo, *hi - 100); - *lo = max(*lo, 100); - } + else + *lo = clamp_val(*lo, 100, *hi - 100); } else { *hi = -1; *lo = -1;