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;
Message-ID: <92e3c2c4-4384-04ae-2016-ad9be8a3fc9b@akamai.com>
Date:   Wed, 7 Sep 2022 10:10:05 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v6 10/57] dyndbg: cleanup auto vars in dynamic_debug_init
Content-Language: en-US
To:     Jim Cromie <jim.cromie@gmail.com>, gregkh@linuxfoundation.org,
        dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
        intel-gvt-dev@lists.freedesktop.org,
        intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc:     daniel.vetter@ffwll.ch, seanpaul@chromium.org, robdclark@gmail.com,
        linux@rasmusvillemoes.dk, joe@perches.com
References: <20220904214134.408619-1-jim.cromie@gmail.com>
 <20220904214134.408619-11-jim.cromie@gmail.com>
From:   Jason Baron <jbaron@akamai.com>
In-Reply-To: <20220904214134.408619-11-jim.cromie@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/4/22 17:40, Jim Cromie wrote:
> rework var-names for clarity, regularity
> rename variables
>   - n to mod_sites - it counts sites-per-module
>   - entries to i - display only
>   - iter_start to iter_mod_start - marks start of each module's subrange
>   - modct to mod_ct - stylistic
> 
> new iterator var:
>   - site - cursor parallel to iter
>     1st step towards 'demotion' of iter->site, for removal later
> 
> treat vars as iters:
>   - drop init at top
>     init just above for-loop, in a textual block
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  lib/dynamic_debug.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index e96dc216463b..2e8ebef3bd0d 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -1059,11 +1059,10 @@ static int __init dynamic_debug_init_control(void)
>  
>  static int __init dynamic_debug_init(void)
>  {
> -	struct _ddebug *iter, *iter_start;
> -	const char *modname = NULL;
> +	struct _ddebug *iter, *iter_mod_start;
> +	int ret, i, mod_sites, mod_ct;
> +	const char *modname;
>  	char *cmdline;
> -	int ret = 0;
> -	int n = 0, entries = 0, modct = 0;
>  
>  	if (&__start___dyndbg == &__stop___dyndbg) {
>  		if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
> @@ -1074,30 +1073,32 @@ static int __init dynamic_debug_init(void)
>  		ddebug_init_success = 1;
>  		return 0;
>  	}
> -	iter = __start___dyndbg;
> +
> +	iter = iter_mod_start = __start___dyndbg;
>  	modname = iter->modname;
> -	iter_start = iter;
> -	for (; iter < __stop___dyndbg; iter++) {
> -		entries++;
> +	i = mod_sites = mod_ct = 0;
> +
> +	for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
> +

Just a small nit here - not sure why there's an extra blank line here?
But either way:

Acked-by: Jason Baron <jbaron@akamai.com>


>  		if (strcmp(modname, iter->modname)) {
> -			modct++;
> -			ret = ddebug_add_module(iter_start, n, modname);
> +			mod_ct++;
> +			ret = ddebug_add_module(iter_mod_start, mod_sites, modname);
>  			if (ret)
>  				goto out_err;
> -			n = 0;
> +
> +			mod_sites = 0;
>  			modname = iter->modname;
> -			iter_start = iter;
> +			iter_mod_start = iter;
>  		}
> -		n++;
>  	}
> -	ret = ddebug_add_module(iter_start, n, modname);
> +	ret = ddebug_add_module(iter_mod_start, mod_sites, modname);
>  	if (ret)
>  		goto out_err;
>  
>  	ddebug_init_success = 1;
>  	vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in __dyndbg section\n",
> -		 entries, modct, (int)((modct * sizeof(struct ddebug_table)) >> 10),
> -		 (int)((entries * sizeof(struct _ddebug)) >> 10));
> +		 i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
> +		 (int)((i * sizeof(struct _ddebug)) >> 10));
>  
>  	/* now that ddebug tables are loaded, process all boot args
>  	 * again to find and activate queries given in dyndbg params.