2023-01-18 13:08:28

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] arch_topology: Build cacheinfo from primary CPU

On Wed, Jan 18, 2023 at 11:55:59AM +0000, Sudeep Holla wrote:
> On Wed, 4 Jan 2023 19:30:23 +0100, Pierre Gondois wrote:
> > v2:
> > - Applied renaming/formatting comments from v1.
> > - Check CACHE_TYPE_VALID flag in pppt.c.
> > v3:
> > - Applied Sudeep's suggestions (for patch 5/5):
> > - Renaming allocate_cache_info() -> fecth_cache_info()
> > - Updated error message
> > - Extract an inline allocate_cache_info() function
> > - Re-run checkpatch with --strict option
> > v4:
> > - Remove RISC-V's implementation of init_cache_level() as not
> > necessary.
> > - Add patch: 'cacheinfo: Check 'cache-unified' property to count
> > cache leaves' to increase the number of leaves at a cache level
> > when no cache-size property is found.
> > - In cacheinfo: Use RISC-V's init_cache_level() [...],
> > make 'levels', 'leaves' and 'level' unsigned int to match
> > of_property_read_u32()'s parameters signedness.
> >
> > [...]
>
> Applied to sudeep.holla/linux (for-next/cacheinfo), thanks!
>

I pushed the changes and then noticed some build warning report by
kbuild posted only to you and one list(missing this list). Please post the
fix if required on top of my for-next/cacheinfo so that it can be added
on the top. Sorry for missing that.

--
Regards,
Sudeep


2023-01-18 15:04:49

by Pierre Gondois

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] arch_topology: Build cacheinfo from primary CPU


On 1/18/23 13:07, Sudeep Holla wrote:
> On Wed, Jan 18, 2023 at 11:55:59AM +0000, Sudeep Holla wrote:
>> On Wed, 4 Jan 2023 19:30:23 +0100, Pierre Gondois wrote:
>>> v2:
>>> - Applied renaming/formatting comments from v1.
>>> - Check CACHE_TYPE_VALID flag in pppt.c.
>>> v3:
>>> - Applied Sudeep's suggestions (for patch 5/5):
>>> - Renaming allocate_cache_info() -> fecth_cache_info()
>>> - Updated error message
>>> - Extract an inline allocate_cache_info() function
>>> - Re-run checkpatch with --strict option
>>> v4:
>>> - Remove RISC-V's implementation of init_cache_level() as not
>>> necessary.
>>> - Add patch: 'cacheinfo: Check 'cache-unified' property to count
>>> cache leaves' to increase the number of leaves at a cache level
>>> when no cache-size property is found.
>>> - In cacheinfo: Use RISC-V's init_cache_level() [...],
>>> make 'levels', 'leaves' and 'level' unsigned int to match
>>> of_property_read_u32()'s parameters signedness.
>>>
>>> [...]
>>
>> Applied to sudeep.holla/linux (for-next/cacheinfo), thanks!
>>
>
> I pushed the changes and then noticed some build warning report by
> kbuild posted only to you and one list(missing this list). Please post the
> fix if required on top of my for-next/cacheinfo so that it can be added
> on the top. Sorry for missing that.
>

Hi Sudeep,
I think the reported issue can be ignored, the 'levels' and 'split_levels'
variables are initialized when used. If necessary, it is straightforward
to fix the warning.
Regards,
Pierre


The reported issue:
--- Start ---

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/base/cacheinfo.c: In function 'fetch_cache_info':
>> drivers/base/cacheinfo.c:440:50: warning: 'levels' is used uninitialized [-Wuninitialized]
440 | this_cpu_ci->num_leaves = levels + split_levels;
| ~~~~~~~^~~~~~~~~~~~~~
drivers/base/cacheinfo.c:420:22: note: 'levels' was declared here
420 | unsigned int levels, split_levels;
| ^~~~~~
>> drivers/base/cacheinfo.c:440:50: warning: 'split_levels' is used uninitialized [-Wuninitialized]
440 | this_cpu_ci->num_leaves = levels + split_levels;
| ~~~~~~~^~~~~~~~~~~~~~
drivers/base/cacheinfo.c:420:30: note: 'split_levels' was declared here
420 | unsigned int levels, split_levels;
| ^~~~~~~~~~~~


vim +/levels +440 drivers/base/cacheinfo.c

416
417 int fetch_cache_info(unsigned int cpu)
418 {
419 struct cpu_cacheinfo *this_cpu_ci;
420 unsigned int levels, split_levels;
421 int ret;
422
423 if (acpi_disabled) {
424 ret = init_of_cache_level(cpu);
425 if (ret < 0)
426 return ret;
427 } else {
428 ret = acpi_get_cache_info(cpu, &levels, &split_levels);
429 if (ret < 0)
430 return ret;
431
432 this_cpu_ci = get_cpu_cacheinfo(cpu);
433 this_cpu_ci->num_levels = levels;
434 /*
435 * This assumes that:
436 * - there cannot be any split caches (data/instruction)
437 * above a unified cache
438 * - data/instruction caches come by pair
439 */
> 440 this_cpu_ci->num_leaves = levels + split_levels;
441 }
442 if (!cache_leaves(cpu))
443 return -ENOENT;
444
445 return allocate_cache_info(cpu);
446 }
447

--- End ---