From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3E813C433F5 for ; Wed, 19 Jan 2022 08:02:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5476610E378; Wed, 19 Jan 2022 08:02:48 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 926AE10E37D; Wed, 19 Jan 2022 08:02:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642579367; x=1674115367; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=ttJ0yEmqdVCXnFR0RzsiyMUeg+GLGi5ZbTTPlF7sNtk=; b=QCm7YCAHyNlWXz1d4yKiZm7vKfYYlsze8V9FvE5AciC09sRjMBu8yK8Y 41zEVo6choBlOFKBAEsrc0A/toQlA0ZHVM5F5lZJ3Pfq0qAfO9gatzesk Mo9VQzH3pkFYrVCK8saKeDjxvhEZBUhK5soDzZNPe7cxuoK9ga74kUvLM 0K+c+X4TLHF/4RK+/HNo6e95WAr3dqRsXzVCd7K1VIKHj4dqYIKwv5fhU wMQP0UsmEoGW6DIe/eilialuBwa1WDpT8bkh5LuRRyYzx3yKAJXCZqdb5 0UzVbcSNYITzrg3LTZzibidFuxx3Zpm8o4lE6cNIPQ6L9ilTQetFOGM4X g==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="243833025" X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="243833025" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:47 -0800 X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="477282343" Received: from elenadel-mobl.ger.corp.intel.com (HELO localhost) ([10.252.50.196]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:33 -0800 From: Jani Nikula To: Lucas De Marchi , linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220119072450.2890107-1-lucas.demarchi@intel.com> Date: Wed, 19 Jan 2022 10:02:31 +0200 Message-ID: <87sftk40h4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Nouveau] [PATCH 0/3] lib/string_helpers: Add a few string helpers X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , Joonas Lahtinen , Rasmus Villemoes , Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Petr Mladek , Sakari Ailus , Leo Li , Steven Rostedt , Julia Lawall , Rahul Lakkireddy , Rodrigo Vivi , Mikita Lipski , Eryk Brol , Greg Kroah-Hartman , Christian =?utf-8?Q?K=C3=B6nig?= , Sergey Senozhatsky , Daniel Vetter , Raju Rangoju , Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.co= m/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@l= inux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in > the tree, with no change in behavior or binary size. For binary > size what I checked wat that the linked objects in the end have the > same size (gcc 11). From comments in the previous attempts this seems > also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew > Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org > because other people argumented in favor of shorter names for these > simple helpers - if they are long and people simply not use due to > that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the > compilations units was one of the concerns and I think re-using this > already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one > > e. One alternative to all of this suggested by Christian K=C3=B6nig > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian K=C3=B6nig > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +----- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++++++----- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ----------- > include/linux/string_helpers.h | 4 ++++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 18 ++++-------------- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) --=20 Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E54DC433EF for ; Wed, 19 Jan 2022 08:03:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351821AbiASIDG (ORCPT ); Wed, 19 Jan 2022 03:03:06 -0500 Received: from mga14.intel.com ([192.55.52.115]:29408 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343971AbiASIC6 (ORCPT ); Wed, 19 Jan 2022 03:02:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642579378; x=1674115378; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=ttJ0yEmqdVCXnFR0RzsiyMUeg+GLGi5ZbTTPlF7sNtk=; b=mYrG9e1ovvkPWdIPpvPg8Wcb91xVxVYzvBRnOaBXEv/WatO+rbVHty+Q qz5hq6PY2pYP4z9wsS/r2rDIEuc9SwbuikVueEVdgKnQxT38agZFYmU6A j5os+aYPLDltT7j14kr9w3cqggXFvtLZ7+r4dzhJL7u82VAF58JXRijhz pGIDftbqzZFsR4i7bhLWpSib/7jIip1WAc0np29DmMpwKUQWkZnEOvvLn Pq9B/l7WIF3XrlvNKz+MY1lsDXsj5hMKfqiTYNJlakSzvn5yn7F7Wg4xZ zoSdgdu1dZM88htGut04hqWi3aoj+L0yywzBPcxbE7QGa3ButDb8QapP6 g==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="245207743" X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="245207743" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:47 -0800 X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="477282343" Received: from elenadel-mobl.ger.corp.intel.com (HELO localhost) ([10.252.50.196]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:33 -0800 From: Jani Nikula To: Lucas De Marchi , linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org Cc: Alex Deucher , Andrew Morton , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Christian =?utf-8?Q?K=C3=B6nig?= , Chris Wilson , Daniel Vetter , David Airlie , "David S . Miller" , Emma Anholt , Eryk Brol , Francis Laniel , Greg Kroah-Hartman , Harry Wentland , Jakub Kicinski , Joonas Lahtinen , Julia Lawall , Kentaro Takeda , Leo Li , Mikita Lipski , Petr Mladek , Rahul Lakkireddy , Raju Rangoju , Rasmus Villemoes , Rodrigo Vivi , Sakari Ailus , Sergey Senozhatsky , Steven Rostedt , Vishal Kulkarni Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220119072450.2890107-1-lucas.demarchi@intel.com> Date: Wed, 19 Jan 2022 10:02:31 +0200 Message-ID: <87sftk40h4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.co= m/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@l= inux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in > the tree, with no change in behavior or binary size. For binary > size what I checked wat that the linked objects in the end have the > same size (gcc 11). From comments in the previous attempts this seems > also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew > Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org > because other people argumented in favor of shorter names for these > simple helpers - if they are long and people simply not use due to > that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the > compilations units was one of the concerns and I think re-using this > already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one > > e. One alternative to all of this suggested by Christian K=C3=B6nig > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian K=C3=B6nig > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +----- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++++++----- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ----------- > include/linux/string_helpers.h | 4 ++++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 18 ++++-------------- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) --=20 Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B28B8C433F5 for ; Wed, 19 Jan 2022 08:02:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 71A6410E386; Wed, 19 Jan 2022 08:02:48 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 926AE10E37D; Wed, 19 Jan 2022 08:02:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642579367; x=1674115367; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=ttJ0yEmqdVCXnFR0RzsiyMUeg+GLGi5ZbTTPlF7sNtk=; b=QCm7YCAHyNlWXz1d4yKiZm7vKfYYlsze8V9FvE5AciC09sRjMBu8yK8Y 41zEVo6choBlOFKBAEsrc0A/toQlA0ZHVM5F5lZJ3Pfq0qAfO9gatzesk Mo9VQzH3pkFYrVCK8saKeDjxvhEZBUhK5soDzZNPe7cxuoK9ga74kUvLM 0K+c+X4TLHF/4RK+/HNo6e95WAr3dqRsXzVCd7K1VIKHj4dqYIKwv5fhU wMQP0UsmEoGW6DIe/eilialuBwa1WDpT8bkh5LuRRyYzx3yKAJXCZqdb5 0UzVbcSNYITzrg3LTZzibidFuxx3Zpm8o4lE6cNIPQ6L9ilTQetFOGM4X g==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="243833025" X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="243833025" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:47 -0800 X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="477282343" Received: from elenadel-mobl.ger.corp.intel.com (HELO localhost) ([10.252.50.196]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:33 -0800 From: Jani Nikula To: Lucas De Marchi , linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220119072450.2890107-1-lucas.demarchi@intel.com> Date: Wed, 19 Jan 2022 10:02:31 +0200 Message-ID: <87sftk40h4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , Rasmus Villemoes , Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Petr Mladek , Sakari Ailus , Leo Li , Steven Rostedt , Julia Lawall , Rahul Lakkireddy , Rodrigo Vivi , Mikita Lipski , Eryk Brol , Greg Kroah-Hartman , Christian =?utf-8?Q?K=C3=B6nig?= , Sergey Senozhatsky , Raju Rangoju , Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.co= m/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@l= inux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in > the tree, with no change in behavior or binary size. For binary > size what I checked wat that the linked objects in the end have the > same size (gcc 11). From comments in the previous attempts this seems > also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew > Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org > because other people argumented in favor of shorter names for these > simple helpers - if they are long and people simply not use due to > that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the > compilations units was one of the concerns and I think re-using this > already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one > > e. One alternative to all of this suggested by Christian K=C3=B6nig > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian K=C3=B6nig > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +----- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++++++----- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ----------- > include/linux/string_helpers.h | 4 ++++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 18 ++++-------------- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) --=20 Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6D5CAC433EF for ; Wed, 19 Jan 2022 08:02:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 67D6A10E37D; Wed, 19 Jan 2022 08:02:48 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 926AE10E37D; Wed, 19 Jan 2022 08:02:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642579367; x=1674115367; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=ttJ0yEmqdVCXnFR0RzsiyMUeg+GLGi5ZbTTPlF7sNtk=; b=QCm7YCAHyNlWXz1d4yKiZm7vKfYYlsze8V9FvE5AciC09sRjMBu8yK8Y 41zEVo6choBlOFKBAEsrc0A/toQlA0ZHVM5F5lZJ3Pfq0qAfO9gatzesk Mo9VQzH3pkFYrVCK8saKeDjxvhEZBUhK5soDzZNPe7cxuoK9ga74kUvLM 0K+c+X4TLHF/4RK+/HNo6e95WAr3dqRsXzVCd7K1VIKHj4dqYIKwv5fhU wMQP0UsmEoGW6DIe/eilialuBwa1WDpT8bkh5LuRRyYzx3yKAJXCZqdb5 0UzVbcSNYITzrg3LTZzibidFuxx3Zpm8o4lE6cNIPQ6L9ilTQetFOGM4X g==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="243833025" X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="243833025" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:47 -0800 X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="477282343" Received: from elenadel-mobl.ger.corp.intel.com (HELO localhost) ([10.252.50.196]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:33 -0800 From: Jani Nikula To: Lucas De Marchi , linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220119072450.2890107-1-lucas.demarchi@intel.com> Date: Wed, 19 Jan 2022 10:02:31 +0200 Message-ID: <87sftk40h4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-gfx] [PATCH 0/3] lib/string_helpers: Add a few string helpers X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , Rasmus Villemoes , Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Petr Mladek , Sakari Ailus , Leo Li , Steven Rostedt , Julia Lawall , Rahul Lakkireddy , Mikita Lipski , Eryk Brol , Greg Kroah-Hartman , Christian =?utf-8?Q?K=C3=B6nig?= , Sergey Senozhatsky , Raju Rangoju , Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.co= m/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@l= inux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in > the tree, with no change in behavior or binary size. For binary > size what I checked wat that the linked objects in the end have the > same size (gcc 11). From comments in the previous attempts this seems > also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew > Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org > because other people argumented in favor of shorter names for these > simple helpers - if they are long and people simply not use due to > that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the > compilations units was one of the concerns and I think re-using this > already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one > > e. One alternative to all of this suggested by Christian K=C3=B6nig > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian K=C3=B6nig > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +----- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++++++----- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ----------- > include/linux/string_helpers.h | 4 ++++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 18 ++++-------------- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) --=20 Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0C37CC433F5 for ; Wed, 19 Jan 2022 08:02:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4500110E39B; Wed, 19 Jan 2022 08:02:49 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 926AE10E37D; Wed, 19 Jan 2022 08:02:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642579367; x=1674115367; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=ttJ0yEmqdVCXnFR0RzsiyMUeg+GLGi5ZbTTPlF7sNtk=; b=QCm7YCAHyNlWXz1d4yKiZm7vKfYYlsze8V9FvE5AciC09sRjMBu8yK8Y 41zEVo6choBlOFKBAEsrc0A/toQlA0ZHVM5F5lZJ3Pfq0qAfO9gatzesk Mo9VQzH3pkFYrVCK8saKeDjxvhEZBUhK5soDzZNPe7cxuoK9ga74kUvLM 0K+c+X4TLHF/4RK+/HNo6e95WAr3dqRsXzVCd7K1VIKHj4dqYIKwv5fhU wMQP0UsmEoGW6DIe/eilialuBwa1WDpT8bkh5LuRRyYzx3yKAJXCZqdb5 0UzVbcSNYITzrg3LTZzibidFuxx3Zpm8o4lE6cNIPQ6L9ilTQetFOGM4X g==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="243833025" X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="243833025" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:47 -0800 X-IronPort-AV: E=Sophos;i="5.88,299,1635231600"; d="scan'208";a="477282343" Received: from elenadel-mobl.ger.corp.intel.com (HELO localhost) ([10.252.50.196]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 00:02:33 -0800 From: Jani Nikula To: Lucas De Marchi , linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220119072450.2890107-1-lucas.demarchi@intel.com> Date: Wed, 19 Jan 2022 10:02:31 +0200 Message-ID: <87sftk40h4.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , Joonas Lahtinen , Rasmus Villemoes , Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Petr Mladek , Sakari Ailus , Leo Li , Steven Rostedt , Julia Lawall , Rahul Lakkireddy , Rodrigo Vivi , Mikita Lipski , Eryk Brol , Greg Kroah-Hartman , Christian =?utf-8?Q?K=C3=B6nig?= , Sergey Senozhatsky , Daniel Vetter , Raju Rangoju , Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" On Tue, 18 Jan 2022, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.co= m/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@l= inux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in > the tree, with no change in behavior or binary size. For binary > size what I checked wat that the linked objects in the end have the > same size (gcc 11). From comments in the previous attempts this seems > also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew > Morton in 20191023155619.43e0013f0c8c673a5c508c1e@linux-foundation.org > because other people argumented in favor of shorter names for these > simple helpers - if they are long and people simply not use due to > that, we failed > > c. Use string_helper.h for these helpers - pulling string.h in the > compilations units was one of the concerns and I think re-using this > already existing header is better than creating a new string-choice.h > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one > > e. One alternative to all of this suggested by Christian K=C3=B6nig > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] > > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. Thanks for picking this up again. I agree with the approach here. Acked-by: Jani Nikula > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. > > thanks > Lucas De Marchi > > Cc: Alex Deucher > Cc: Andrew Morton > Cc: Andy Shevchenko > Cc: Andy Shevchenko > Cc: Ben Skeggs > Cc: Christian K=C3=B6nig > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: David Airlie > Cc: David S. Miller > Cc: Emma Anholt > Cc: Eryk Brol > Cc: Francis Laniel > Cc: Greg Kroah-Hartman > Cc: Harry Wentland > Cc: Jakub Kicinski > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Julia Lawall > Cc: Kentaro Takeda > Cc: Leo Li > Cc: Mikita Lipski > Cc: Petr Mladek > Cc: Rahul Lakkireddy > Cc: Raju Rangoju > Cc: Rasmus Villemoes > Cc: Rodrigo Vivi > Cc: Sakari Ailus > Cc: Sergey Senozhatsky > Cc: Steven Rostedt > Cc: Vishal Kulkarni > > Lucas De Marchi (3): > lib/string_helpers: Consolidate yesno() implementation > lib/string_helpers: Add helpers for enable[d]/disable[d] > drm: Convert open yes/no strings to yesno() > > drivers/gpu/drm/amd/amdgpu/atom.c | 3 ++- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6 +----- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_dp_helper.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 4 +++- > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++++++----- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 3 ++- > .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ----------- > include/linux/string_helpers.h | 4 ++++ > security/tomoyo/audit.c | 2 +- > security/tomoyo/common.c | 18 ++++-------------- > security/tomoyo/common.h | 1 - > 15 files changed, 31 insertions(+), 59 deletions(-) --=20 Jani Nikula, Intel Open Source Graphics Center